PyPi Simple endpoint end slash handling

[dlouzan]

What Renovate type, platform and version are you using?

Self-hosted latest

Describe the bug

At the moment I'm not sure if this is an actual bug, but wanted to open a discussion about it.

While combining renovate self-hosted with JFrog's artifactory and the PyPi Simple index, renovate is not being able to find dependencies on Artifactory due to handling of the end slash in the index endpoint.

The issue happens in combination with https://www.jfrog.com/jira/browse/RTFACT-14235, which basically is a problem on Artifactory redirecting HTTPS to HTTP (yeah 😒). So for instance:

# This works
curl -v https://internal.domain/artifactory/api/pypi/pypi-all/simple/

# This is redirected to HTTP with slash at the end
curl -v https://internal.domain/artifactory/api/pypi/pypi-all/simple
...
< HTTP/1.1 302 Found
< Date: Mon, 22 Feb 2021 19:36:40 GMT
< Server: Artifactory/x.x.x
< X-Artifactory-Id: xxx
< Location: http://internal.domain/artifactory/api/pypi/pypi-all/simple/
< Content-Length: 0

While this looks to me an artifactory issue, the relevant PEP itself points to all addresses ending in slash, so at the moment I'm not sure where the issue really is. Should per spec all URLs end in slash?

But I'm relatively sure, if renovate appended the slash, everything would work because there wouldn't be any redirects involved 🙇

I'd be happy for some guidance or opinions.

/cc @max-wittig @fh1ch @nejch

[dlouzan]

The PEP says:

All URLs which respond with an HTML5 page MUST end with a / and the repository SHOULD redirect the URLs without a / to add a / to the end.

While Artifactory clearly behaves incorrectly here by redirecting an HTTPS to HTTP, I think renovate would be safe to append / ensure / to all simple requests?

[github-actions]

This issue has been labeled with status:requirements. This indicates that we don't know enough to start work and further requirements or a reproduction are needed first.

This label will be replaced with status:ready once all requirements and reproductions necessary to start work have been met.

If it's not clear what is missing to move this issue forward, ask for clarification in a new comment. If you think we already have what we need to move forward, mention this in a new comment.

[viceice]

I think this can be easily fixed by ensureEndingSlash on lookupName here:

renovate/lib/datasource/pypi/index.ts

Line 251 in 94be71c

dependency = await getSimpleDependency(lookupName, hostUrl);

[rarkins]

Where is the URL coming from? e.g. Renovate config, requirements.txt, etc?

Is it the plain registry url that's getting a 302, or one which we construct with dependency name too?

[viceice]

Url is constructed here:

renovate/lib/datasource/pypi/index.ts

Lines 178 to 180 in 94be71c

	const lookupUrl = url.resolve(hostUrl, `${packageName}`);
	const dependency: ReleaseResult = { releases: null };
	const response = await http.get(lookupUrl);

so packageName needs an ending slash here.

[nejch]

Just one thing to keep in mind also is that some proxies don't handle double/multiple slashes very well (mostly Apache I think, resulting in 404), so it should only append if needed. I know python-gitlab is doing the opposite (stripping trailing slashes from base urls) for this reason (python-gitlab/python-gitlab#1027).

[viceice]

Just one thing to keep in mind also is that some proxies don't handle double/multiple slashes very well (mostly Apache I think, resulting in 404), so it should only append if needed. I know python-gitlab is doing the opposite (stripping trailing slashes from base urls) for this reason (python-gitlab/python-gitlab#1027).

That's why we have a util function for it 😉

renovate/lib/util/url.ts

Lines 3 to 5 in d8df51f

	export function ensureTrailingSlash(url: string): string {
	return url.replace(/\/?$/, '/');
	}