Closed Bug 1818374 Opened 1 year ago Closed 10 months ago

Most URL property setters should strip tabs, newlines, carriage returns

Categories

(Core :: DOM: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
118 Branch
Tracking Status
firefox112 --- wontfix
firefox118 --- fixed

People

(Reporter: canadahonk, Assigned: dotoole)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files, 1 obsolete file)

Most (specifically: protocol, host, hostname, port, pathname, search, hash) property setters should strip certain whitespace (tabs, newlines, carriage returns). Some (protocol, search, hash) are already done but the majority (host, hostname, port, pathname) don't.

WPT tests: https://github.com/web-platform-tests/wpt/blob/master/url/url-setters-stripping.any.js

Related to Bug 1812060 - but generalised to all setters.

Not sure how the best way to go about this is, just doing the same approach as https://phabricator.services.mozilla.com/D170404 for all the setters missing?

I assume so. It would probably be nice to have a method that does all this. Probably belongs in https://searchfox.org/mozilla-central/source/netwerk/base/nsURLHelper.h

Severity: -- → S3
Priority: -- → P2
Whiteboard: [necko-triaged]
Assignee: nobody → dotoole

(In reply to CanadaHonk [:canadahonk] from comment #0)

Most (specifically: protocol, host, hostname, port, pathname, search, hash) property setters should strip certain whitespace (tabs, newlines, carriage returns). Some (protocol, search, hash) are already done but the majority (host, hostname, port, pathname) don't.

WPT tests: https://github.com/web-platform-tests/wpt/blob/master/url/url-setters-stripping.any.js

Related to Bug 1812060 - but generalised to all setters.

Hello, I've made some progress fixing host,hostname,pathname but am stuck trying to fix port. Could you point me to the right place to do the stripping as stripping the input for SetPort or SetHostPort does not seem to affect the wpt tests? Thanks

I don't really touch Necko much and don't know about those. You might want to try the #necko channel on Matrix (chat.mozilla.org).

Attachment #9341787 - Attachment description: WIP: Bug 1818374 - strip whitespaces for URL property setters → Bug 1818374 - strip whitespaces for URL property setters. r=kershaw
Attachment #9344459 - Attachment description: Bug 1818374 - strip whitespaces for URL property setters. r=kershaw → Bug 1818374 - strip whitespaces for URL property setters.
Attachment #9344459 - Attachment is obsolete: true
Attachment #9341787 - Attachment description: Bug 1818374 - strip whitespaces for URL property setters. r=kershaw → WIP: Bug 1818374 - strip whitespaces for URL property setters
Attachment #9341787 - Attachment description: WIP: Bug 1818374 - strip whitespaces for URL property setters → Bug 1818374 - strip whitespaces for URL property setters. r=kershaw

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:dotoole, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Flags: needinfo?(kershaw)
Flags: needinfo?(dotoole)
Flags: needinfo?(kershaw)

These strings may contain embedded NUL characters, including in initial position,
so it's incorrect to test for !*string_ptr and assume this means it can be rejected
as empty. Instead, we should check the string object for emptiness.

(Noticed this bug when I was looking over the current URL failures in interop-2023.)

Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/978ce5bde0ea
strip whitespaces for URL property setters. r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/a3a9cd66f483
followup - Fix incorrect check for empty string in nsStandardURL::SetFilePath and SetQueryWithEncoding. r=necko-reviewers,valentin
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
Flags: needinfo?(dotoole)
Blocks: 1849332
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: