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

RESOLVED FIXED in mozilla20

Status

Core Graveyard
File Handling
--
minor
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: Julian Reschke, Assigned: Julian Reschke)

Tracking

(Blocks: 1 bug)

unspecified
mozilla20

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → julian.reschke
(Assignee)

Comment 1

6 years ago
Created attachment 646423 [details] [diff] [review]
Proposed patch, incl test case
Attachment #646423 - Flags: review?(jduell.mcbugs)
(Assignee)

Comment 2

6 years ago
Created attachment 686064 [details] [diff] [review]
Proposed patch, incl test case

Patch updated to apply to latest trunk; try results at https://tbpl.mozilla.org/?tree=Try&rev=28e559bfc67a
Attachment #646423 - Attachment is obsolete: true
Attachment #646423 - Flags: review?(jduell.mcbugs)
Attachment #686064 - Flags: review?(jduell.mcbugs)
(Assignee)

Comment 3

6 years ago
Created attachment 686083 [details] [diff] [review]
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)
Attachment #686064 - Attachment is obsolete: true
Attachment #686064 - Flags: review?(jduell.mcbugs)
Attachment #686083 - Flags: review?(jduell.mcbugs)
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!
Attachment #686083 - Flags: review?(jduell.mcbugs) → review+

Comment 5

6 years ago
https://hg.mozilla.org/mozilla-central/rev/575d6637b9f1
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
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).
(Assignee)

Comment 7

6 years ago
(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...)
> Citation needed.

bug 579517.

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

  https://hg.mozilla.org/integration/mozilla-inbound/rev/972aa16a72e7
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.