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

[feature] Pass original URL as a header when downloading sources from backup URLs #17420

Open
1 task done
VinayVi opened this issue Dec 5, 2024 · 4 comments
Open
1 task done
Assignees
Milestone

Comments

@VinayVi
Copy link

VinayVi commented Dec 5, 2024

What is your suggestion?

When conan downloads sources from the backup URL, I would like the original URL to be passed as a header. Maybe something like X-Resource-Original-Url: https://example.com/libfoo.tar.gz.

This can be done with a one line change right here: https://sourcegraph.com/github.com/conan-io/conan/-/blob/conans/client/downloaders/caching_file_downloader.py?L114-118

headers = {'X-Resource-Original-Url': urls}
self._file_downloader.download(backup_url + sha256, cached_path, sha256=sha256, headers=headers)

My use case is that my company prevents access to the public internet by default except for specifically permitted services. If we pass this header, then all downloads can go through a service that acts as a proxy.

I've created a service that acts as a source caching proxy. When a request is made to myservice.com/, first I check if that sha has already been downloaded, and if it has return that file immediately. If it hasn't been downloaded yet, then it reads the headers for the original URL, downloads the file, verifies it with the passed in sha, and then returns it to the client. It's an extremely simple service and this proxy means that normal users don't need to access the rest of the internet.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@VinayVi
Copy link
Author

VinayVi commented Dec 5, 2024

FWIW I got this working by patching Conan locally and testing it against my server. The one weird bit that I ran into was that the .json endpoint needed to have the right structure, or be missing completely for it to work (see this line here: https://sourcegraph.com/github.com/conan-io/conan/-/blob/conans/client/downloaders/download_cache.py?L112-126). I got it working by just commenting out the line where the .json file was downloaded. I would appreciate it if there was some way to skip that json download file, or at least be able to return an empty {} file so that it gets regenerated every time.

@memsharded
Copy link
Member

Hi @VinayVi

Thanks for your suggestion.

While I understand the purpose, I think this is slightly not connected to the current sources-caching feature. If this is implemented server side, then there is no need to require a Conan client cache. It might be good and useful, but not necessary. So the feature could be designed from a different perspective.

Though on the other hand, one of the advantages of the current proposal is that it needs someone to actively download then upload to the server. And this is good, it requires a explicit audit only by authorized person of the external sources used, some of our users that use this feature like it precisely because it won't allow developers to download any arbitrary sources from the internet and build them, as that is also a security risk.

We'll try to think about this possibility, but I am afraid this wouldn't be a high priority at this moment (we are focused on other parts as Workspaces and new CMakeDeps), I am labeling this as 2.X future.

@memsharded memsharded added this to the 2.X milestone Dec 5, 2024
@VinayVi
Copy link
Author

VinayVi commented Dec 5, 2024

@memsharded
I understand that it's a low priority from your POV; this is something that I can work on. If I made the PR with the changes (just the two lines I suggested above), would it get merged or blocked?

The changes that I'm suggesting should be 100% backwards compatible just because it's adding a brand new header, so IMO it's very safe to do.

@memsharded
Copy link
Member

We don't know yet if this would be merged or blocked, it is not only about the risk, but about the architecture, design and maintenance of the application. Conan could add a myriad of no-risk extra things that each one would serve to one user. While nothing would break, everything has to be tested, documented and maintained, and that has always an impact. So we need to be careful what gets in into the application.

This extra header, while low risk and easy to implement, could open the door to further questions, usages and maintenance, because there is another side, a server side that needs to understand, process and maintain that. Then is when derivative feature requests, questions, issues, etc follow up.

Some quick questions about the header:

  • Is this header a "standard" or common header, or a custom header?
  • If it is a standard one, please provide some links or reference
  • This might sound specific to Conan protocol of the backup-sources, why is it not a X-Conan header?

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

No branches or pull requests

2 participants