Failing test: anchor lost in redirection

[mro]

#308 (comment)

[ArthurHoaro]

There are 2 issues here:

• Anchor is lost while saving a new link because the referer points to do=addlink which we prevent. That's a bug.
• Your tests are failing because generateLocation() doesn't accept relative URLs as referer. Should it though?

[mro]

Anchor is lost while saving a new link because the referer points to do=addlink which we prevent.

can't follow. The referer doesn't mention do=addlink. However, I as well consider the new test to uncover an old bug.

Your tests are failing because generateLocation() doesn't accept relative URLs as referer. Should it though?

I think so. If I understand

Shaarli/index.php

Line 1358 in 38bedfb

$returnurl = ( !empty($_POST['returnurl']) ? escape($_POST['returnurl']) : '?' );

correctly, it's actually used with relative URLs.

I have no idea what generateLocation is supposed to precisely do, however. The comments are not enlightning.

[ArthurHoaro]

can't follow. My test case doesn't mention do=addlink.

No, I was talking about a manual test: add a new link to see if I get the anchor back.

If I understand correctly, it's actually used with relative URLs.

$_POST['returnurl'] is set with HTTP referer which should be a full URL. But that part is a bit messy, I agree and I missed it in 775803a.

[mro]

a manual test: add a new link to see if I get the anchor back.

done there: #308 (comment) (both manual and scripted)

$_POST['returnurl'] is set with HTTP referer

No, lin 1358 reads to me:

$returnurl = ( !empty($_POST['returnurl']) ? escape($_POST['returnurl']) : '?' );

so it is set either with the content of returnurl (which can be just anything) or ? which isn't an absolute url either.

Are we reading different code? Did you look at line 1358 mentioned above?

[mro]

a side note: why does @virtualtam lock conversations like e.g. at #353 (comment) ?

[ArthurHoaro]

Here. $returnurl is set to ? only if the referer is empty (which doesn't really make sense, I agree).

This thread is meant to be an "official" release thread, and is locked to be kept clean.

[mro]

so after all the answer to "should it work with relative URLs" is IMO "yes", true?

doesn't really make sense

IMO it makes perfect sense - it was like this for years and worked fine.

clean

So where can I contribute to your comment #353 (comment) ?

[mro]

(oh - and the HTTP_REFERER is unreliable and may contain about just anything sent by the HTTP-client, right?)

[ArthurHoaro]

so after all the answer to "should it work with relative URLs" is IMO "yes", true?

Yes, probably.

So where can I contribute to your comment #353 (comment) ?

#308 if it's a general question, any specific issue thread (or create a new one) for specific matters.

(oh - and the HTTP_REFERER is unreliable and may contain about just anything sent by the browser, right?)

Yup. https://github.com/shaarli/Shaarli/wiki/Troubleshooting#redirection-issues-http-referer

[mro]

code part of ArthurHoaro@d01c234#diff-226b7ed8aece0926bdd4a67e6ece8903R121 as part of #368