Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curl: make --xattr save the original URL #9768

Closed
wants to merge 2 commits into from
Closed

Conversation

bagder
Copy link
Member

@bagder bagder commented Oct 19, 2022

No description provided.

@bagder
Copy link
Member Author

bagder commented Oct 19, 2022

This causes a minor peculiarity: imagine we get URL A that redirects to B. This now saves the URL as A (formerly it used B).

The xattr code also saves the referer header. But the rreferer header only makes sense after a redirect, so right now this gets the referer from the second request... which happens to always be A if automatic referer is used!

@vszakats
Copy link
Member

vszakats commented Oct 19, 2022

True. Well, auto-referrer might trigger to save the effective URL to the origin xattr, but this seems way more trouble than it's worth.

Perhaps the best solution is to just not use automatic referrers.

[ Off-topic: A generic solution could be a --write-out-like option where the user could customize what information to write to which xattr, e.g. --xattr-out 'user.xdg.origin.url=%{url_effective}'. New variables, like download timestamp or hash can be added to the list. --xattr would trigger the default set. ]

@bagder
Copy link
Member Author

bagder commented Oct 20, 2022

@mback2k the Appveyor builds now all appear as failed, but looking at the details there is no link to figure out more!

It appears as if it still shows the previous rounds build failure while in fact it has restarted a new build after the PR was updated. Can this be it?

@bagder
Copy link
Member Author

bagder commented Oct 20, 2022

No that was not it. It seems they were failed builds, but without links to the build logs.

@bagder
Copy link
Member Author

bagder commented Oct 21, 2022

Right, the new test does not work on Windows...

@vszakats
Copy link
Member

vszakats commented Oct 21, 2022

You can just disable these tests for Windows. On Windows there are no xattrs. There is a similar feature called file streams, but there is no fsetxattr() API for it.

@bagder
Copy link
Member Author

bagder commented Oct 21, 2022

I figure there are more platforms than Windows without it, so I tried to make it a proper feature for the test suite to require.

@bagder
Copy link
Member Author

bagder commented Oct 21, 2022

Since #9769: while appveyor builds are running they show as failed jobs (with the red cross), and if you click details it shows the wrong commit (message)

@mback2k
Copy link
Member

mback2k commented Oct 21, 2022

I will look at the AppVeyor issue tonight.

@bagder

This comment was marked as outdated.

mback2k added a commit that referenced this pull request Oct 21, 2022
mback2k added a commit to libssh2/libssh2 that referenced this pull request Oct 21, 2022
Adjusted test 1621 accordingly.

Reported-by: Viktor Szakats
Fixes #9766
Closes #9768

fixup update unit 1621
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants