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

Server uses wrong path logic when extracting from tarball #9

Open
joachimvh opened this issue Apr 12, 2022 · 2 comments
Open

Server uses wrong path logic when extracting from tarball #9

joachimvh opened this issue Apr 12, 2022 · 2 comments

Comments

@joachimvh
Copy link
Member

When the server receives a path such as http://localhost:3000/bundles/npm/@solid/community-server/3.0.0/components/context.jsonld, it has to download the tarball and look in there to find that exact resource. It does this by taking the tail, components/context.jsonld, and looking for that path. The problem is that this doesn't work if there is no direct mapping between the URL tail and the file path. The package above, for example, has the following in it's package.json:

  "lsd:importPaths": {
    "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^3.0.0/components/": "dist/components/",
    "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^3.0.0/config/": "config/",
    "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^3.0.0/dist/": "dist/",
    "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^3.0.0/templates/config/": "templates/config/"
  }

So it should actually find dist/components/context.jsonld. This requires checking if the incoming URL starts with one of the import paths, or is at least equal by using semver rules on the version number, notice the difference between 3.0.0 and ^3.0.0 above. It then has to check that the trailing part of the URL matches the trailing part of one of the importPath URLs. And then it has to replace the overlapping part with the value of the map. This makes the operation quite a bit more complicated but is feasible to do.

The issue that I have, and why I'm tagging @rubensworks, is because there are 2 other possible ways to configure the package.json and I'm not sure what is expected there.

First case is having "lsd:module": true in your config. How is the server supposed to know how to map the URL to a file path in the tarball? I had a look at the example in the documentation, but that caused more confusion since I have no idea how the equivalence there is achieved: https://componentsjs.readthedocs.io/en/latest/getting_started/basics/exposing_components/#packagejson-discovery

The other case is when there are only lsd:contexts entries. What should be done in that case? Or should only the exact URLs there be supported and mapped to their values, while other paths should be ignored?

@rubensworks
Copy link
Member

This requires checking if the incoming URL starts with one of the import paths, or is at least equal by using semver rules on the version number, notice the difference between 3.0.0 and ^3.0.0 above. It then has to check that the trailing part of the URL matches the trailing part of one of the importPath URLs. And then it has to replace the overlapping part with the value of the map. This makes the operation quite a bit more complicated but is feasible to do.

I guess some semver-equivalence mapping is required here indeed.

First case is having "lsd:module": true in your config. How is the server supposed to know how to map the URL to a file path in the tarball?

I would suggest just calling this function: https://github.com/LinkedSoftwareDependencies/Components.js/blob/master/lib/loading/ModuleStateBuilder.ts#L175-L220

The other case is when there are only lsd:contexts entries. What should be done in that case? Or should only the exact URLs there be supported and mapped to their values, while other paths should be ignored?

Then there should be no mapping I guess? Are there cases where only lsd:contexts is present? Because I'm not even sure CJS supports that if no importPaths are present.

@joachimvh
Copy link
Member Author

Are there cases where only lsd:contexts is present? Because I'm not even sure CJS supports that if no importPaths are present.

No idea. The comment above that block implies it's there for backwards compatibility.

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

2 participants