Loose base URL check can crash URL parser

[rubycon]

Loose base URL check can crash URL parser

[domenic]

I think the bug here is a bit more subtle. It should not be possible to set init["baseURL"] to null because its type is USVString, not USVString?. But I guess the spec does that in "initialize" step 2.3.

So we should add a guard to step 2.3 at least.

We may also need an empty string guard. I am less sure about that.

I also think that using map exists to check if values are properly set is maybe not the most robust practice as map exists can be true even if the value associated with the key is null or undefined.

This is not quite right in our case. Because the dictionary member is defined to be of type USVString, it cannot hold undefined or null values. So "map exists" is good enough and no more checks should be necessary.

[sisidovski]

I agree, 11. If init["baseURL"] exists: in process a URLPatternInit should also check if it's a non-empty string.

[sisidovski]

I think the bug here is a bit more subtle. It should not be possible to set init["baseURL"] to null because its type is USVString, not USVString?. But I guess the spec does that in "initialize" step 2.3.

I overlooked the type. Yes, it's USVString so it can't be null. Thanks.

How about adding an empty string guard to between step 2.2 and 2.3 and throwing a TypeError?

[rubycon]

You're right @domenic, type already should be insured. However there's other places where null values are explicitely set URLPatternInit entries.

During initialization process a URLPatternInit is called with all url parts argument to null ! The function allow null to be passed for these arguments but they are only use to fill an URLPatternInit struct.

[jeremyroman]

I don't follow the argument for a non-empty string guard. Of course this is going to fail ultimately, but it seems fine for it to do so in "process a URLPatternInit" 11.2 (after we fail to parse the empty string as a URL).