search

adamdruppe / arsd

This is a collection of modules that I've released over the years. Most of them stand alone, or have just one or two dependencies in here, so you don't have to download this whole repo.
http://arsd-official.dpldocs.info/arsd.html
530 stars 127 forks source link

Fix populateFromInfo on URIs relative to previous host:port #277

Closed AlexAltea closed 3 years ago

AlexAltea commented 3 years ago

Sometimes the response Location header is /aaa/bbb rather than https://host:port/aaa/bbb. This makes sure the provided URI is absolute before copying any data.

adamdruppe commented 3 years ago

OK, yeah, that looks good. I seem to recall that's a violation of the http spec but I might be wrong and even so if people do it I guess we should handle it.

I might also use the URI.basedOn function which does all the relative handling (just like navigateTo) here.

Do you want the fix somewhat urgently? If no I'll take a look later to check a few more cases, if yes though I can merge and even tag another bug fix release now though.

AlexAltea commented 3 years ago

Relative locations are allowed by the spec, cf. https://tools.ietf.org/html/rfc7231#section-7.1.2

7.1.2. Location

The "Location" header field is used in some responses to refer to a specific resource in relation to the response. The type of relationship is defined by the combination of request method and status code semantics.

 Location = URI-reference

The field value consists of a single URI-reference. When it has the form of a relative reference ([RFC3986], Section 4.2), the final value is computed by resolving it against the effective request URI ([RFC3986], Section 5).

AlexAltea commented 3 years ago

I might also use the URI.basedOn function which does all the relative handling (just like navigateTo) here.

Yeah, that sounds much better! I didn't know where to get an Uri instance for the original request.

Do you want the fix somewhat urgently?

It's slightly urgent, as it's breaking some scripts, but I guess I can find find a temporary workaround in the meantime.

adamdruppe commented 3 years ago

On Fri, Feb 26, 2021 at 07:23:51AM -0800, Alexandro Sanchez wrote:

Yeah, that sounds much better! I didn't know where to get an Uri instance for the original request.

if there isn't one I'll add one.

It's slightly urgent, as it's breaking some scripts, but I guess I can find find a temporary workaround in the meantime.

I'm just putting out another fire first then will return so give me a lil bit.

adamdruppe commented 3 years ago 
            auto parts = where.basedOn(this.where);
            this.where = parts;

I actually think throwing that into popupatFromInfo is going to do the trick. I'm doing my tests now. But if this.where is empty or if the new where is absolute, it uses the new one, otherwise it adapts these.

Gonna make a dummy server to actually confirm now then i'll push and tag.

adamdruppe commented 3 years ago

Yeah it seems to work well. gonna close this and push that.

thanks for your help!