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

Correct calculation of metadata file URL #12034

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

pfmoore
Copy link
Member

@pfmoore pfmoore commented May 17, 2023

Fixes #12033

Still needs tests.

@property
def metadata_file_url(self) -> str:
scheme, netloc, path, query, fragment = self._parsed_url
return urllib.parse.urlunsplit((scheme, netloc, path + ".metadata", query, ""))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep the query? In #12033 the query is used to implement Base64 checking, and including the query here might throw off the server. The spec doesn’t really comment about this, but since the original query is for the original URL, it’s reasonable to not include it for the new path.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea. It's just as plausible that the query could contain an access token that allows you to fetch data from the server.

I think the only way we'd know for sure what the correct behaviour is would be to survey what existing index server implementations do. And if there's none currently using the query segment, I'd be tempted to amend the PEP to prohibit the query segment in index URLs altogether.

@sbidoul
Copy link
Member

sbidoul commented Oct 1, 2023

Is this still needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PEP658 metadata URL calculation bug
4 participants