Follow method validation rules in Request constructor

RESOLVED FIXED in mozilla38

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: nsm, Assigned: nsm)

Tracking

33 Branch
mozilla38
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Step 14 of the request constructor is not implemented properly currently.
Created attachment 8549831 [details] [diff] [review]
Follow method validation rules when constructing Request
Attachment #8549831 - Flags: review?(bkelly)
Assignee: nobody → nsm.nikhil
Status: NEW → ASSIGNED

Comment 2

4 years ago
Comment on attachment 8549831 [details] [diff] [review]
Follow method validation rules when constructing Request

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

The normalization of just some headers confused me, but that is indeed what the spec says.  LGTM.  Thanks!

Over to Andrea as DOM peer.
Attachment #8549831 - Flags: review?(bkelly)
Attachment #8549831 - Flags: review?(amarchesini)
Attachment #8549831 - Flags: review+
Comment on attachment 8549831 [details] [diff] [review]
Follow method validation rules when constructing Request

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

::: dom/fetch/Request.cpp
@@ +155,2 @@
>    if (aInit.mMethod.WasPassed()) {
>      nsCString method = aInit.mMethod.Value();

nsAutoCString

@@ +157,4 @@
>  
> +    // Step 14.1. Disallow forbidden methods, and anything that is not a HTTP
> +    // token, since HTTP states that Method may be any of the defined values or
> +    // a token (extension method).

I still prefer the lowerCase approach. These are several string operations and for each one you ask to convert the method to lower case.
Can you do something similar to:

nsAutoString lowerCaseMethod;
nsContentUtils::ASCIIToLower(aInit.mMethod.Value(), lowerCaseMethod);
Attachment #8549831 - Flags: review?(amarchesini) → review+
I Used an uppercase approach instead since normalization is uppercasing.
https://hg.mozilla.org/mozilla-central/rev/c2c0d8f26b97
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Duplicate of this bug: 1123587

Comment 8

3 years ago
Comment on attachment 8549831 [details] [diff] [review]
Follow method validation rules when constructing Request

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

::: dom/fetch/Request.cpp
@@ +160,5 @@
> +    // a token (extension method).
> +    if (method.LowerCaseEqualsLiteral("connect") ||
> +        method.LowerCaseEqualsLiteral("trace") ||
> +        method.LowerCaseEqualsLiteral("track") ||
> +        !NS_IsValidHTTPToken(method)) {

Missing #include "nsNetUtil.h"

Updated

3 years ago
Depends on: 1125588
You need to log in before you can comment on or make changes to this bug.