Closed Bug 777687 Opened 8 years ago Closed 8 years ago

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

Categories

(Core Graveyard :: File Handling, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla20

People

(Reporter: julian.reschke, Assigned: julian.reschke)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 2 obsolete files)

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: nobody → julian.reschke
Attached patch Proposed patch, incl test case (obsolete) — Splinter Review
Attachment #646423 - Flags: review?(jduell.mcbugs)
Attached patch Proposed patch, incl test case (obsolete) — Splinter Review
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)
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+
https://hg.mozilla.org/mozilla-central/rev/575d6637b9f1
Status: NEW → RESOLVED
Closed: 8 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).
(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.