1122194 - Follow method validation rules in Request constructor

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Step 14 of the request constructor is not implemented properly currently.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Follow method validation rules when constructing Request

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 3

[Andrea Marchesini [:baku]]

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);

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

I Used an uppercase approach instead since normalization is uppercasing.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2c0d8f26b97

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c2c0d8f26b97

Comment 8

[Yuan Pengfei]

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"