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

Allow serving cabal files with package name in the file name. #1262

Merged
merged 1 commit into from Dec 4, 2023
Merged

Allow serving cabal files with package name in the file name. #1262

merged 1 commit into from Dec 4, 2023

Conversation

ghost
Copy link

@ghost ghost commented Oct 26, 2023

@ghost ghost mentioned this pull request Oct 26, 2023
@@ -456,6 +458,11 @@ coreFeature ServerEnv{serverBlobStore = store} UserFeature{..}
resourceDesc = [(GET, "Get package .cabal file revision")]
, resourceGet = [("cabal", serveCabalFileRevision)]
}
coreCabalFileRevName = (resourceAt "/package/:package/revision/:package-:revision.:format") {
Copy link
Contributor

@phadej phadej Oct 26, 2023

Choose a reason for hiding this comment

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

I wonder how :package...:package template works. It probably doesn't (i.e. allows non-sense like /package/foo/revision/bar-1-r2.cabal, or even /package/non---sense-1-1-1-1/revision/bar-1-r2.cabal

See e.g

    corePackageTarball = (resourceAt "/package/:package/:tarball.tar.gz") {
   --- which uses:
    packageTarballInPath dpath = do
      PackageIdentifier name version <- packageInPath dpath
      case lookup "tarball" dpath >>= fromReqURI of
        Nothing -> mzero
        Just pkgid@(PackageIdentifier name' version') -> do
          -- rules:
          -- \* the package name and tarball name must be the same
          -- \* the tarball must specify a version
          -- \* the package must either have no version or the same version as the tarball
          guard $ name == name' && version' /= nullVersion && (version == version' || version == nullVersion)
          return pkgid

Copy link
Author

Choose a reason for hiding this comment

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

Why is that a problem? If someone is using a malformed url, they will get malformed data. I.e. it's their problem.

Is there a security issue here I don't see?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a security issue here I don't see?

There is CDN in from of hackage-server which caches the results. I'm sure someone can be annoying, as there would be infinitely many cacheable urls. Maybe not a security risk, but in my opinion wrong URLs should be reported as the wrong ones.

Copy link
Author

@ghost ghost Oct 26, 2023

Choose a reason for hiding this comment

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

Okay. Then I don't know how to implement it. I can monkey-copy the code that closely matches someone else's, but I don't know how to implement unification in Haskell.

In the defence of my, undoubtedly, flawed, position, I can say that github also allows infinitely many cacheable URLs, like https://github.com/username/project/archive/v1.0.0/bar-$version.tag.gz

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I don't know how to implement it.

I gave you a pointer. Look how corePackageTarball is implemented (using packageTarballInPath which you can use as an example).

Copy link
Author

Choose a reason for hiding this comment

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

Please theck the new version. I have reused the "tarball" function, because, as far as I understand, it is essentially the same.

But maybe I misunderstood.

Copy link
Author

Choose a reason for hiding this comment

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

@phadej does the new version make sense? Although I re-used the function with "tar" in the name, it seems that the pattern match is the same.

@ghost
Copy link
Author

ghost commented Nov 2, 2023

What do the failures with those two older compilers mean?

@gbaz gbaz merged commit 22b5612 into haskell:master Dec 4, 2023
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

Successfully merging this pull request may close these issues.

Unique filenames for $revision.cabal.
2 participants