Code Monkey home page Code Monkey logo

Comments (16)

tibbe avatar tibbe commented on July 17, 2024

It unclear to me what relativeTo does. The documentation states that it returns "an absolute URI" but this can't be if both URIs are relative, as an absolute URI would have to include a scheme, which we won't have if both URIs are relative.

from network.

singpolyma avatar singpolyma commented on July 17, 2024

Yeah. Perhaps it should return a Maybe for some possible fail cases (like both are relative), unless you can resove one relative one in relation to a less relative one, in which case the docs are wrong but that wouldn't need a Maybe return type. The implementation right now, though, always returns a Just, so there's no need for the Maybe

from network.

tibbe avatar tibbe commented on July 17, 2024

I'd prefer if we didn't break clients (by removing the Maybe) until we've figured out what we want this function to do. :)

from network.

singpolyma avatar singpolyma commented on July 17, 2024

That's fair.

If I may ask a historical question: who wrote the code? (That is: Why is it that you don't know what it does?)

I would be OK with either implementation (require that the result be absolute and use a Maybe or else make it so that it just references one based on another and does not return a Maybe)

from network.

tibbe avatar tibbe commented on July 17, 2024

I inherited the network package (which is very old) at some point. I didn't write any of the code in Network.URI. You can try to use git blame to dig around in the version history and see if there's a commit message that can shine some light on why the function behaves like it does.

from network.

singpolyma avatar singpolyma commented on July 17, 2024

git show df60c2ae shows the first imported implementation of relativeTo (the type signature line has not changed since then, says git blame). At that time there was a code path that retuned Nothing, but the code has changed quite a bit since then.

from network.

tibbe avatar tibbe commented on July 17, 2024

At this point I'm not sure how to proceed. Do you want to try to change the meaning of relativeTo? It seems to me that it must return Maybe given the two relative URIs case I mentioned.

from network.

singpolyma avatar singpolyma commented on July 17, 2024

The code claims to implement the algorithm from http://tools.ietf.org/html/rfc3986#section-5.2, I am looking into this algorithm now.

from network.

singpolyma avatar singpolyma commented on July 17, 2024

Reading over the algorithm, indeed the return value is not a Maybe, but nor is it guarenteed to be an absolute URI. The documentation of the function as it exists right now is wrong. The algorithm takes two URIs, either or both of which may be absolute or relative. It resolves the one in the context of the other, and returns the result, which will be an absolute URI if the base was and otherwise will not be. If the second URI is an absolute URI, the result is just a normalisation pass on the input URI, without looking at the base.

I think this algorithm is sensible, and the current implementation looks right, but it has no reason to return a Maybe

from network.

tibbe avatar tibbe commented on July 17, 2024

To check how many packages would be affected by a change in relativeTo's type, I did a grep of hackage:

./Webrexp/1.1/Webrexp-1.1/Text/Webrexp/ResourcePath.hs:58:    case b `relativeTo` a of
./Webrexp/1.1/Webrexp-1.1/Text/Webrexp/ResourcePath.hs:66:        Just r -> Remote . fromJust $ r `relativeTo` a
./hmarkup/3000.0.1/hmarkup-3000.0.1/Text/HMarkup/XHtml.hs:125:                   Just baseURI -> uri `relativeTo'` baseURI
./hmarkup/3000.0.1/hmarkup-3000.0.1/Text/HMarkup/XHtml.hs:126:  where relativeTo' x y = 
./hmarkup/3000.0.1/hmarkup-3000.0.1/Text/HMarkup/XHtml.hs:127:            fromMaybe (error $ "relativeTo " ++ show x ++ " " ++ show y) 
./hmarkup/3000.0.1/hmarkup-3000.0.1/Text/HMarkup/XHtml.hs:128:                          (x `relativeTo` y)
./cabal-install/0.14.0/cabal-install-0.14.0/Distribution/Client/BuildReports/Upload.hs:18:import Network.URI (URI, uriPath, parseRelativeReference, relativeTo)
./cabal-install/0.14.0/cabal-install-0.14.0/Distribution/Client/BuildReports/Upload.hs:54:                                     relativeTo rel uri
./windowslive/0.3/windowslive-0.3/src/Network/WindowsLive/ConsentToken.hs:86:--   in relConsentUrl \`relativeTo\` 'consentUrl'
./windowslive/0.3/windowslive-0.3/test/Test.hs:9:import Network.URI ( relativeTo, uriQuery )
./windowslive/0.3/windowslive-0.3/test/Test.hs:80:      uri <- case relLoginUri `relativeTo` baseUrl of
./hxt-filter/8.4.2/hxt-filter-8.4.2/src/Text/XML/HXT/Parser/XmlInput.hs:58:    , relativeTo
./hxt-filter/8.4.2/hxt-filter-8.4.2/src/Text/XML/HXT/Parser/XmlInput.hs:290:         abs'  <- relativeTo uri' base'
./doc-review/0.7.1/doc-review-0.7.1/src/Main.hs:19:                                              , relativeTo
./doc-review/0.7.1/doc-review-0.7.1/src/Main.hs:173:  -- relativeTo's implementation always returns Just
./doc-review/0.7.1/doc-review-0.7.1/src/Main.hs:174:  redirect $ B.pack $ show $ fromJust $ d `relativeTo` u
./doc-review/0.7.1/doc-review-0.7.1/src/Main.hs:243:  makeAbsolute <- flip relativeTo <$> baseURL
./xml-catalog/0.8.0/xml-catalog-0.8.0/Text/XML/Catalog.hs:51:                            case parseURIReference (withBase ref) >>= flip relativeTo uri of
./xml-catalog/0.8.0/xml-catalog-0.8.0/Text/XML/Catalog.hs:58:                            case parseURIReference (withBase ref) >>= flip relativeTo uri of
./xml-catalog/0.8.0/xml-catalog-0.8.0/Text/XML/Catalog.hs:65:                            case parseURIReference (withBase catalog) >>= flip relativeTo uri of
./xml-catalog/0.8.0/xml-catalog-0.8.0/Text/XML/Catalog.hs:90:                    ref `relativeTo` base
./extemp/0.0.1/extemp-0.0.1/Extemp.hs:34:import Network.URI (URI(..),URIAuth(..), relativeTo, parseURI, parseURIReference, uriQuery)
./extemp/0.0.1/extemp-0.0.1/Extemp.hs:134:      r <- x `relativeTo` u
./hask-home/2009.3.18/hask-home-2009.3.18/hask-home.hs:135:fileURI desc f = fromJust $ (nullURI { uriPath = f }) `relativeTo` darcsURI desc
./ObjectIO/1.0.1.1/ObjectIO-1.0.1.1/Graphics/UI/ObjectIO/Window/Validate.hs:239:        = (if hasWindowHandlesWindow (toWID relativeTo) windows then Just itemPos else Nothing,atts')
./ObjectIO/1.0.1.1/ObjectIO-1.0.1.1/Graphics/UI/ObjectIO/Window/Validate.hs:243:        (isRelative,relativeTo)  = isRelativeItemPos itemPos
./ObjectIO/1.0.1.1/ObjectIO-1.0.1.1/Graphics/UI/ObjectIO/Window/Validate.hs:526:                        (wptr,wsH1)  = case find (unidentifyWindow (toWID relativeTo)) wsHs of
./ObjectIO/1.0.1.1/ObjectIO-1.0.1.1/Graphics/UI/ObjectIO/Window/Validate.hs:572:                (isRelative,relativeTo) = isRelativeItemPos itemPos
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Distribution/Client/BuildReports/Upload.hs:18:import Network.URI (URI, uriPath, parseRelativeReference, relativeTo)
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Distribution/Client/BuildReports/Upload.hs:54:                                     relativeTo rel uri
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/Browser.hs:125:   , parseURI, parseURIReference, relativeTo
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/Browser.hs:914:                   newURI_abs = maybe newURI id (newURI `relativeTo` uri)
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/Browser.hs:1036:uriDefaultTo a b = maybe a id (a `relativeTo` b)
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/URI.hs:84:    , relativeTo
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/URI.hs:940:--  > "foo" `relativeTo` "http://bar.org/" = "http://bar.org/foo"
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/URI.hs:947:nonStrictRelativeTo ref base = relativeTo ref' base
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/URI.hs:958:relativeTo :: URI -> URI -> Maybe URI
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/URI.hs:959:relativeTo ref base
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/URI.hs:1051:--  > (uabs `relativeFrom` ubase) `relativeTo` ubase == uabs
./cabal-install-bundle/0.14.0/cabal-install-bundle-0.14.0/Network/HTTP/Auth.hs:192:        annotateURIs = (map (\u -> fromMaybe u (u `relativeTo` baseURI))) . catMaybes
./network/2.3.1.0/network-2.3.1.0/Network/URI.hs:84:    , relativeTo
./network/2.3.1.0/network-2.3.1.0/Network/URI.hs:942:--  > "foo" `relativeTo` "http://bar.org/" = "http://bar.org/foo"
./network/2.3.1.0/network-2.3.1.0/Network/URI.hs:949:nonStrictRelativeTo ref base = relativeTo ref' base
./network/2.3.1.0/network-2.3.1.0/Network/URI.hs:960:relativeTo :: URI -> URI -> Maybe URI
./network/2.3.1.0/network-2.3.1.0/Network/URI.hs:961:relativeTo ref base
./network/2.3.1.0/network-2.3.1.0/Network/URI.hs:1053:--  > (uabs `relativeFrom` ubase) `relativeTo` ubase == uabs
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:35:    , relativeTo, nonStrictRelativeTo
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:441:        mkabs (Just u1) (Just u2) = shabs (u1 `relativeTo` u2)
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:640:   difficulty in constructinmg a reversible relativeTo/relativeFrom pair of functions.
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:1120:-- Test strict vs non-strict relativeTo logic
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:1127:      (fromJust $ parseURIReference "foo") `relativeTo` trbase)
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:1132:      (fromJust $ parseURIReference "http:foo") `relativeTo` trbase)
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:1229:cu02 = ou02 `relativeTo` bu02
./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:1272:-- Added some additional test cases raised by discussion on [email protected] mailing list about 2005-07-19.  The test p[roposed by this discussion exposed a subtle bug in relativeFrom not being an exact inverse of relativeTo.
./git-annex/3.20120807/git-annex-3.20120807/Utility/Url.hs:111:                         newURI_abs = fromMaybe newURI (newURI `relativeTo` u)
./Agda/2.3.0.1/Agda-2.3.0.1/src/full/Agda/TypeChecking/Monad/Options.hs:157:setIncludeDirs incs relativeTo = do
./Agda/2.3.0.1/Agda-2.3.0.1/src/full/Agda/TypeChecking/Monad/Options.hs:161:  (root, check) <- case relativeTo of
./cabal-install-ghc74/0.10.4/cabal-install-ghc74-0.10.4/Distribution/Client/BuildReports/Upload.hs:18:import Network.URI (URI, uriPath, parseRelativeReference, relativeTo)
./cabal-install-ghc74/0.10.4/cabal-install-ghc74-0.10.4/Distribution/Client/BuildReports/Upload.hs:56:                                     relativeTo rel uri
./SpaceInvaders/0.4.2/SpaceInvaders-0.4.2/src/RenderLandscape.hs:46:dmPoints = map relativeToGPoint
./SpaceInvaders/0.4.2/SpaceInvaders-0.4.2/src/RenderLandscape.hs:62:cmPoints = map relativeToGPoint
./SpaceInvaders/0.4.2/SpaceInvaders-0.4.2/src/RenderLandscape.hs:72:groundPoints = map relativeToGPoint
./SpaceInvaders/0.4.2/SpaceInvaders-0.4.2/src/RenderLandscape.hs:80:relativeToGPoint :: (Double, Double) -> HGL.Point
./SpaceInvaders/0.4.2/SpaceInvaders-0.4.2/src/RenderLandscape.hs:81:relativeToGPoint (x,y) = (round (x * fromIntegral worldSizeX),
./hackport/0.2.18/hackport-0.2.18/cabal/cabal-install/Distribution/Client/BuildReports/Upload.hs:18:import Network.URI (URI, uriPath, parseRelativeReference, relativeTo)
./hackport/0.2.18/hackport-0.2.18/cabal/cabal-install/Distribution/Client/BuildReports/Upload.hs:56:                                     relativeTo rel uri
./HTTP/4000.2.3/HTTP-4000.2.3/Network/Browser.hs:125:   , parseURI, parseURIReference, relativeTo
./HTTP/4000.2.3/HTTP-4000.2.3/Network/Browser.hs:914:                   newURI_abs = maybe newURI id (newURI `relativeTo` uri)
./HTTP/4000.2.3/HTTP-4000.2.3/Network/Browser.hs:1036:uriDefaultTo a b = maybe a id (a `relativeTo` b)
./HTTP/4000.2.3/HTTP-4000.2.3/Network/HTTP/Auth.hs:192:        annotateURIs = (map (\u -> fromMaybe u (u `relativeTo` baseURI))) . catMaybes
./approximate-equality/1.1/approximate-equality-1.1/Data/Eq/Approximate.hs:114:    relativeToleranceOf ::
./approximate-equality/1.1/approximate-equality-1.1/Data/Eq/Approximate.hs:276:            else show x ++ " +/- " ++ show (x * relativeToleranceOf a * 100)
./approximate-equality/1.1/approximate-equality-1.1/Data/Eq/Approximate.hs:294:        reltol = relativeToleranceOf a
./approximate-equality/1.1/approximate-equality-1.1/Data/Eq/Approximate.hs:426:    relativeToleranceOf =
./LinkChecker/0.1/LinkChecker-0.1/LinkChecker.hs:107:            makeUpURI string mapping = ((`relativeTo` snd mapping) . fromMaybe nullURI . parseURIReference) $ drop (length $ fst mapping) string
./LinkChecker/0.1/LinkChecker-0.1/LinkChecker.hs:108:        baseURI = fromMaybe nullURI (fromMaybe nullURI (parseAbsoluteURI $ fromMaybe "" b) `relativeTo` implicitBaseURI)
./LinkChecker/0.1/LinkChecker-0.1/LinkChecker.hs:122:            requestURI = fromJust $ fromJust maybeURI `relativeTo` baseURI -- relativeTo never returns Nothing. wtf
./uri-enumerator/0.1.0.1/uri-enumerator-0.1.0.1/Network/URI/Enumerator.hs:16:    , relativeTo
./uri-enumerator/0.1.0.1/uri-enumerator-0.1.0.1/Network/URI/Enumerator.hs:135:relativeTo :: URI -> URI -> Maybe URI
./uri-enumerator/0.1.0.1/uri-enumerator-0.1.0.1/Network/URI/Enumerator.hs:136:relativeTo a b = fmap fromNetworkURI $ toNetworkURI a `N.relativeTo` toNetworkURI b
./webdriver/0.4/webdriver-0.4/src/Test/WebDriver/Internal.hs:52:    (Just baseURI, Just relURI) -> return . fromJust $ relURI `relativeTo` baseURI
./swish/0.8.0.0/swish-0.8.0.0/src/Swish/RDF/Parser/Utils.hs:71:import Network.URI (URI(..), relativeTo, parseURIReference)
./swish/0.8.0.0/swish-0.8.0.0/src/Swish/RDF/Parser/Utils.hs:84:--   `Network.URI.relativeTo`.
./swish/0.8.0.0/swish-0.8.0.0/src/Swish/RDF/Parser/Utils.hs:91:    "" -> case uri `relativeTo` base of
./http-conduit/1.6.0/http-conduit-1.6.0/Network/HTTP/Conduit/Request.hs:36:import Network.URI (URI (..), URIAuth (..), parseURI, relativeTo, escapeURIString, isAllowedInURI)
./http-conduit/1.6.0/http-conduit-1.6.0/Network/HTTP/Conduit/Request.hs:69:    case uri `relativeTo` getUri req of
./uri-conduit/0.5.0/uri-conduit-0.5.0/Network/URI/Conduit.hs:16:    , relativeTo
./uri-conduit/0.5.0/uri-conduit-0.5.0/Network/URI/Conduit.hs:142:relativeTo :: URI -> URI -> Maybe URI
./uri-conduit/0.5.0/uri-conduit-0.5.0/Network/URI/Conduit.hs:143:relativeTo a b = fmap fromNetworkURI $ toNetworkURI a `N.relativeTo` toNetworkURI b
./cabal-install-ghc72/0.10.4/cabal-install-ghc72-0.10.4/Distribution/Client/BuildReports/Upload.hs:18:import Network.URI (URI, uriPath, parseRelativeReference, relativeTo)
./cabal-install-ghc72/0.10.4/cabal-install-ghc72-0.10.4/Distribution/Client/BuildReports/Upload.hs:56:                                     relativeTo rel uri
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly.hs:49:         , absPath, (</>), (<.>), canonic, canonicalize, relPath, relativeTo, path
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Base.hs:16:    eitherRelativeTo, relativeTo, maybeRelativeTo,
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Base.hs:108:relativeTo :: FilePath -- ^ anchor path, the prefix
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Base.hs:111:relativeTo relativeFP fp =
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Base.hs:205:               return (relativeTo wd)
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Pipe.hs:65:         , absPath, (</>), (<.>), canonic, canonicalize, relPath, relativeTo
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Pipe.hs:458:relativeTo :: FilePath -- ^ anchor path, the prefix
./shelly/0.14.0.1/shelly-0.14.0.1/Shelly/Pipe.hs:461:relativeTo = sh2 S.relativeTo
./hums/0.4.3/hums-0.4.3/src/URIExtra.hs:56:    foldl (\a r -> fromJust $ r `relativeTo` a)

(Note that since this a text-based search there might be some false positives in the list above.)

There are some packages that would break. It might be worth looking at some of them to see how they use relativeTo and in particular how they handle a Nothing return value.

This comment might be of interest:

./network/2.3.1.0/network-2.3.1.0/tests/uri001.hs:1272:-- Added some additional test cases raised by discussion on [email protected] mailing list about 2005-07-19.  The test p[roposed by this discussion exposed a subtle bug in relativeFrom not being an exact inverse of relativeTo.

from network.

singpolyma avatar singpolyma commented on July 17, 2024

Ok, sweet. I'll take a look at some of those in more detail. We could maybe also contact some of those authors and ask what they expect the function to do.

We could, for compatability, build another function (resolvedWith or something) and set relativeTo a = Just . resolvedWith a

from network.

singpolyma avatar singpolyma commented on July 17, 2024

I haven't looked at them all yet, but initially:

Webrexp - error/fromJust on Nothing, should work on non-absolute input
hmarkup - error on Nothing, current algo fine
cabal-install - pattern match fail on Nothing, expects absolute output on absolute base (current algo fine)
windowslive - only uses in tests and comments, expects absolute output on absolute base (current algo fine)
hxt-filter - empty string (as indication of error, returned from a function that's spec'd to return either an absolute URI or an empty string) on Nothing, expects absolute output on absolute base (current algo fine)
doc-review - uses fromJust, comments indicate that the author was aware that relativeTo always returs Just
xml-catalog - skips entity processing on Nothing
extemp - Not sure what it does or expects, may need more review, only one version and no update since 2010
hask-home - uses fromJust on Nothing, current algo fine
ObjectIO - false positive, does not depend on network
cabal-install-bundle - fromMaybe u (relativeTo u base) BUT it actually includes the Network.URI source directly and does not depend on network

from network.

igfoo avatar igfoo commented on July 17, 2024

I just ran into this too, while looking at LinkChecker.

That seems to me like a very small number of affected packages in the grand scheme of things, but the sooner the API is fixed the fewer packages will be written to use the incorrect API.

In the case of LinkChecker, building with recent GHC is broken anyway, as it uses both haskell98 and base.

from network.

tibbe avatar tibbe commented on July 17, 2024

@singpolyma Can you look at 4a3fdef to make sure we agree what the changed implementation should do? Also, if you have suggestions for the docs that'd be great.

from network.

singpolyma avatar singpolyma commented on July 17, 2024

I agree with both the code and the documetation change

from network.

tibbe avatar tibbe commented on July 17, 2024

@singpolyma Thanks for checking. Will be in the next major release.

from network.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.