777687 - make handling of broken %-escapes in RFC2231/5987 encoding more draconian

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Julian Reschke]

Test cases:

  http://greenbytes.de/tech/tc2231/#attwithfn2231nbadpct1

and

  http://greenbytes.de/tech/tc2231/#attwithfn2231nbadpct2

The values contain "%" characters that do not start a %hh sequence. Safari 6 now implements RFC 5987 and treats these encoding errors as fatal (so has Konqueror for some time).

Firefox should match this behavior.

Comment 1

[Julian Reschke]

Attachment: Proposed patch, incl test case

Comment 2

[Julian Reschke]

Attachment: Proposed patch, incl test case

Patch updated to apply to latest trunk; try results at https://tbpl.mozilla.org/?tree=Try&rev=28e559bfc67a

Comment 3

[Julian Reschke]

Attachment: Proposed patch, incl test case

Patch updated to apply to latest trunk; try results at https://tbpl.mozilla.org/?tree=Try&rev=325e5cfbe50d (this patch differs from the previous only in whitespace removal)

Comment 4

[Jason Duell]

Comment on attachment 686083 [details] [diff] [review]
Proposed patch, incl test case

Review of attachment 686083 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

  https://hg.mozilla.org/integration/mozilla-inbound/rev/575d6637b9f1

thanks Julian!

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/575d6637b9f1

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 686083 [details] [diff] [review]
Proposed patch, incl test case

Review of attachment 686083 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/mime/nsMIMEHeaderParamImpl.cpp
@@ +146,5 @@
> +
> +// validate that a C String containing %-escapes is syntactically valid
> +bool IsValidPercentEscaped(const char *aValue, PRInt32 len)
> +{
> +  for (PRInt32 i = 0; i < len; i++) {

Hey guys, for future reference, this needs to use int32_t (though uint32_t is actually more appropriate).

Comment 7

[Julian Reschke]

(In reply to :Ms2ger from comment #6)
> Hey guys, for future reference, this needs to use int32_t (though uint32_t
> is actually more appropriate).

Citation needed. (Always willing to learn...)

Comment 8

[Jason Duell]

> Citation needed.

bug 579517.

I've fixed this--thanks for heads-up ms2ger. 

  https://hg.mozilla.org/integration/mozilla-inbound/rev/972aa16a72e7

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/972aa16a72e7