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

Content is not allowed in prolog #355

Open
basilevs opened this issue Oct 19, 2023 · 5 comments
Open

Content is not allowed in prolog #355

basilevs opened this issue Oct 19, 2023 · 5 comments

Comments

@basilevs
Copy link
Contributor

basilevs commented Oct 19, 2023

When fetching repository from https://github.com/fabioz/Pydev/releases/download/pydev_10_1_3 P2 is redirected from

https://github.com/fabioz/Pydev/releases/download/pydev_10_1_3/content.jar

to

https://objects.githubusercontent.com/github-production-release-asset-2e65be/7935003/a5422aac-de32-40ef-a51c-5a186359ece6?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20231019%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20231019T220152Z&X-Amz-Expires=300&X-Amz-Signature=7eb7ccbbd0c4871c6d3337de6de58be22466c44bf4276565a61f839659335df5&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=7935003&response-content-disposition=attachment%3B%20filename%3Dcontent.jar&response-content-type=application%2Foctet-stream

it is confused by the complexity of the resulting URL and fails to recognize content as JAR.

Please see eclipse-tycho/tycho#2938 for steps to reproduce.
Happens only in Tycho, PDE seems to work fine.

@basilevs
Copy link
Contributor Author

basilevs commented Oct 19, 2023

I suspect the line

is using wrong input to detect the JAR extension - localFile is mangled by injected CacheManager.

@merks
Copy link
Contributor

merks commented Oct 20, 2023

It also works in the repository explorer

image

This suggests there is not p2 problem but rather a Tycho problem or a problem in how Tycho uses p2.

@laeubi
Copy link
Member

laeubi commented Oct 20, 2023

This suggests there is not p2 problem but rather a Tycho problem or a problem in how Tycho uses p2.

Well the problem is that P2 assumes that a file that ends with .jar is a jar file and everything else is an xml file, what works for some but not all cases.

Since we have a buffered stream here, it would be better to look at the magic bytes what would be 50 4B in this case instead of rely on the name of a file.

@basilevs
Copy link
Contributor Author

basilevs commented Oct 20, 2023

Indeed, modern Tycho customizes CacheManager and returns a File with a name that represents final redirect location. A fix on Tycho side could be to copy the from redirected name to original name.
However, SimpleMetadataRepositoryFactory should not rely on cache implementation and use storage location in internal logic. Such strong coupling makes injection of custom CacheManager too hard.

basilevs added a commit to basilevs/tycho that referenced this issue Oct 21, 2023
P2 relies on correct file extensions to parse cached files.

Fixes eclipse-tycho#2938

See
eclipse-equinox/p2#355
basilevs added a commit to basilevs/tycho that referenced this issue Oct 21, 2023
P2 relies on correct file extensions to parse cached files.

Fixes eclipse-tycho#2938

See P2 bug:
eclipse-equinox/p2#355

Integration test infrastructure touch-ups:

Fix HttpServer concurrency bug.
URLs returned from HttpServer.getAccessedUrls() are now stripped of
context prefix. No callers have used these values until now.
Fix concurrency bug in test utility class HttpServer request logging.
laeubi pushed a commit to eclipse-tycho/tycho that referenced this issue Oct 22, 2023
P2 relies on correct file extensions to parse cached files.

Fixes #2938

See P2 bug:
eclipse-equinox/p2#355

Integration test infrastructure touch-ups:

Fix HttpServer concurrency bug.
URLs returned from HttpServer.getAccessedUrls() are now stripped of
context prefix. No callers have used these values until now.
Fix concurrency bug in test utility class HttpServer request logging.
@basilevs
Copy link
Contributor Author

basilevs added a commit to basilevs/tycho that referenced this issue Oct 22, 2023
This is a backport from 5.0.0

P2 relies on correct file extensions to parse cached files.
This change prevents cached file name from changing over HTTP redirect.

See
eclipse-equinox/p2#355

Fix HttpServer concurrency bug.
URLs returned from HttpServer.getAccessedUrls() are now stripped of
context prefix. No callers have used these values until now.
Fix concurrency bug in test utility class HttpServer request logging.
Add verification of request logs to ensure that metadata repository does
indeed unjars fresh, non-cached artifact. Ensure that cached XML file
does not produce false negative in tests.
Refactor tests.
laeubi pushed a commit to eclipse-tycho/tycho that referenced this issue Oct 28, 2023
This is a backport from 5.0.0

P2 relies on correct file extensions to parse cached files.
This change prevents cached file name from changing over HTTP redirect.

See
eclipse-equinox/p2#355

Fix HttpServer concurrency bug.
URLs returned from HttpServer.getAccessedUrls() are now stripped of
context prefix. No callers have used these values until now.
Fix concurrency bug in test utility class HttpServer request logging.
Add verification of request logs to ensure that metadata repository does
indeed unjars fresh, non-cached artifact. Ensure that cached XML file
does not produce false negative in tests.
Refactor tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants