This repository has been archived by the owner on Nov 24, 2018. It is now read-only.
OOM killed when using headers with file with relative import #60
Comments
|
I can reproduce: I'm investigating why |
|
Oh, so the issue here is due to custom headers being used to configure their own import, which triggers an infinite recursion. Let me see if this is simple to fix |
Gabriella439
added a commit
to dhall-lang/dhall-haskell
that referenced
this issue
Oct 4, 2018
Fixes dhall-lang/dhall-json#60 In the above issue, a remote import specified custom headers using a relative path: ``` $ curl https://gist.githubusercontent.com/reactormonk/a8192fe784608f1f7515d5583c448095/raw/d46f5b9aea9eb56c4bdd92d187936ee87e9f4cf2/local.dhall let test = https://gist.githubusercontent.com/reactormonk/a8192fe784608f1f7515d5583c448095/raw/e8e5b14c2afe9acc342fe111bcf6e09b90dced9c/test.dhall using ./headers in test ``` But import chaining was not working correctly for the `using` clause of the import, meaning that before this change the `./headers` import was not anchored to the parent import. Using the `</>` judgment from the standard, the previous behavior was: ``` http://example0.com/foo </> http://example1.com/bar using ./baz = http://example1.com/bar using ./baz ``` ... when the behavior should have been: ``` http://example0.com/foo </> http://example1.com/bar using ./baz = http://example1.com/bar using http://example0.com/baz ``` Note that custom header support has not yet been fully standardized, but the latter version is what the correct behavior should be once it is standardized. The reason the former version is problematic is because it does not behave correctly in the presence of header forwarding. If `http://example1.com/bar` imports any remote expressions of its own, it will use the relative import `./baz` to customize the headers for downstream imports from the same domain. However, because this relative import is not anchored to the parent import, it will change for each downstream import. For example, suppose that the contents of `http://example1.com/bar using ./baz` are: ``` http://example1.com/baz ``` Import chaining that after its parent import would give: ``` http://example1.com/bar using ./baz </> http://example1.com/baz = http://example1.com/baz using ./baz ``` ... which introduces an inadvertent import cycle because now `./baz` will resolve to `http://example1.com/baz` instead of `http://example0.com/baz`. An import cycle similar to this one caused the bug described by the linked issue. This change fixes import chaining to correctly anchor custom header imports to the parent import and this fix resolves the above issue. This also includes a small change to fix canonicalization to also canonicalize the custom header import, although this is unrelated to the above bug.
|
I found the bug. The fix is up here: dhall-lang/dhall-haskell#618 |
Gabriella439
added a commit
to dhall-lang/dhall-haskell
that referenced
this issue
Oct 4, 2018
Fixes dhall-lang/dhall-json#60 In the above issue, a remote import specified custom headers using a relative path: ``` $ curl https://gist.githubusercontent.com/reactormonk/a8192fe784608f1f7515d5583c448095/raw/d46f5b9aea9eb56c4bdd92d187936ee87e9f4cf2/local.dhall let test = https://gist.githubusercontent.com/reactormonk/a8192fe784608f1f7515d5583c448095/raw/e8e5b14c2afe9acc342fe111bcf6e09b90dced9c/test.dhall using ./headers in test ``` But import chaining was not working correctly for the `using` clause of the import, meaning that before this change the `./headers` import was not anchored to the parent import. Using the `</>` judgment from the standard, the previous behavior was: ``` http://example0.com/foo </> http://example1.com/bar using ./baz = http://example1.com/bar using ./baz ``` ... when the behavior should have been: ``` http://example0.com/foo </> http://example1.com/bar using ./baz = http://example1.com/bar using http://example0.com/baz ``` Note that custom header support has not yet been fully standardized, but the latter version is what the correct behavior should be once it is standardized. The reason the former version is problematic is because it does not behave correctly in the presence of header forwarding. If `http://example1.com/bar` imports any remote expressions of its own, it will use the relative import `./baz` to customize the headers for downstream imports from the same domain. However, because this relative import is not anchored to the parent import, it will change for each downstream import. For example, suppose that the contents of `http://example1.com/bar using ./baz` are: ``` http://example1.com/baz ``` Import chaining that after its parent import would give: ``` http://example1.com/bar using ./baz </> http://example1.com/baz = http://example1.com/baz using ./baz ``` ... which introduces an inadvertent import cycle because now `./baz` will resolve to `http://example1.com/baz` instead of `http://example0.com/baz`. An import cycle similar to this one caused the bug described by the linked issue. This change fixes import chaining to correctly anchor custom header imports to the parent import and this fix resolves the above issue. This also includes a small change to fix canonicalization to also canonicalize the custom header import, although this is unrelated to the above bug.
|
Could you cut a new binary release? |
|
@reactormonk: We're not ready to cut a release yet because there are some in-flight changes to the standard. However, you can download a prerelease build of http://hydra.dhall-lang.org/job/dhall-json/master/tarball/latest |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Reproduce:
Download https://gist.github.com/reactormonk/a8192fe784608f1f7515d5583c448095
Enjoy OOM!
The text was updated successfully, but these errors were encountered: