read-write-web / rww-play

read write web Play
59 stars 19 forks source link

Relative instead of absolute graph is used in ReadWriteWebControllerGeneric to write responses #149

Closed reederz closed 9 years ago

reederz commented 9 years ago

Here's the line in question: https://github.com/read-write-web/rww-play/blob/dev/app/controllers/ldp/ReadWriteWebControllerGeneric.scala#L210 And here's how CORS proxy writes responses: https://github.com/read-write-web/rww-play/blob/dev/app/rww/play/CORSProxy.scala#L128

Notice that CORS proxy uses absolute graph from ldpr whereas the ReadWriteWebControllerController uses relative graph.

I don't know if changing ReadWriteWebController to use absolute graph instead of relative graph would have some unintended consequences, but it fixes my issue with messed up subjects/objects when requesting resources with "Accept: application/n-triples" header.

Using relative graph in ReadWriteWebControllerGeneric (messed up response):

[  7:42PM ]  [ justas@choedankal:~/Source/scala/workspace/jolocom/rww-play/test_www(jolocom✗) ]
 $ curl -k -i -H "Accept: application/n-triples" -X GET \
   --cert ../eg/test-localhost.pem:test https://localhost:8443/2013/couch
HTTP/1.1 200 OK
Accept-Patch: application/sparql-update
Access-Control-Allow-Origin: *
Allow: OPTIONS, GET, HEAD, SEARCH, PUT, DELETE, PUT, PATCH
Content-Type: application/n-triples
ETag: "1425638154104|Success(857)"
Last-Modified: Fri, 06 Mar 2015 10:35:54 GMT
Link: <http://www.w3.org/ns/ldp#Resource>; rel=type, <couch.acl>; rel=acl
Content-Length: 1635

_:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#city> "Fontainebleau"^^<http://www.w3.org/2001/XMLSchema#string> .
_:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#country> "France"^^<http://www.w3.org/2001/XMLSchema#string> .
<http://localhost:8888/#mycouch> <http://purl.org/goodrelations/v1#description> "Comfortable couch in Artist Stables"^^<http://www.w3.org/2001/XMLSchema#string> .
_:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#postalCode> "77300"^^<http://www.w3.org/2001/XMLSchema#string> .
_:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#street> "21 rue Saint Honoré"^^<http://www.w3.org/2001/XMLSchema#string> .
<http://localhost:8888/> <http://www.w3.org/ns/ldp#contains> <http://localhost:8888/> .
_:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx1 <http://www.w3.org/2000/10/swap/pim/contact#address> _:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx2 .
<http://localhost:8888/#mycouch> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <http://bblfish.net/2013/05/10/couch#Surf> .
<http://localhost:8888/#mycouch> <http://purl.org/goodrelations/v1#color> "Red"^^<http://www.w3.org/2001/XMLSchema#string> .
<http://localhost:8888/#mycouch> <http://purl.org/goodrelations/v1#location> _:genid-34ea3f3a0d2c4c83950c25dfeb57b753-node19fn63u4nx1 .
<http://localhost:8888/#mycouch> <http://purl.org/goodrelations/v1#name> "Very Unstable Couch"^^<http://www.w3.org/2001/XMLSchema#string> .

Using relative graph in ReadWriteWebControllerGeneric (good response):

[  7:54PM ]  [ justas@choedankal:~/Source/scala/workspace/jolocom/rww-play/test_www(jolocom✗) ]
 $ curl -k -i -H "Accept: application/n-triples" -X GET \
   --cert ../eg/test-localhost.pem:test https://localhost:8443/2013/couch
HTTP/1.1 200 OK
Accept-Patch: application/sparql-update
Access-Control-Allow-Origin: *
Allow: OPTIONS, GET, HEAD, SEARCH, PUT, DELETE, PUT, PATCH
Content-Type: application/n-triples
ETag: "1425638154104|Success(857)"
Last-Modified: Fri, 06 Mar 2015 10:35:54 GMT
Link: <http://www.w3.org/ns/ldp#Resource>; rel=type, <couch.acl>; rel=acl
Content-Length: 1712

_:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#city> "Fontainebleau"^^<http://www.w3.org/2001/XMLSchema#string> .
<https://localhost:8443/2013/couch#mycouch> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <http://bblfish.net/2013/05/10/couch#Surf> .
_:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx1 <http://www.w3.org/2000/10/swap/pim/contact#address> _:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx2 .
_:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#street> "21 rue Saint Honoré"^^<http://www.w3.org/2001/XMLSchema#string> .
<https://localhost:8443/2013/couch#mycouch> <http://purl.org/goodrelations/v1#name> "Very Unstable Couch"^^<http://www.w3.org/2001/XMLSchema#string> .
<https://localhost:8443/2013/couch#mycouch> <http://purl.org/goodrelations/v1#color> "Red"^^<http://www.w3.org/2001/XMLSchema#string> .
_:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#country> "France"^^<http://www.w3.org/2001/XMLSchema#string> .
<https://localhost:8443/2013/couch#mycouch> <http://purl.org/goodrelations/v1#location> _:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx1 .
<https://localhost:8443/2013/couch#mycouch> <http://purl.org/goodrelations/v1#description> "Comfortable couch in Artist Stables"^^<http://www.w3.org/2001/XMLSchema#string> .
_:genid-31d5de6955a34663b90f43dfd6d7f322-node19fn63u4nx2 <http://www.w3.org/2000/10/swap/pim/contact#postalCode> "77300"^^<http://www.w3.org/2001/XMLSchema#string> .
<https://localhost:8443/2013/couch> <http://www.w3.org/ns/ldp#contains> <https://localhost:8443/2013/couch> .

I can make a PR for this but I don't know if I wouldn't be breaking something else with it.

bblfish commented 9 years ago

thanks. Can you narrow down on the problem? That will help for documentation of the bug, and help me read quickly what the problem is.

reederz commented 9 years ago

OK. So the fix which you did for https://github.com/w3c/banana-rdf/issues/239 worked. Now, when we serialize graph to application/n-triples format, we get absolute URLs instead of relative URLs. The thing is, that the serializer takes in the wrong base. Above, you can see that we get http://localhost:8888/#mycouch (wrong) instead of https://localhost:8443/2013/couch#mycouch (right).

It seemed to me that http://localhost:8888/ is some sort of default base URL. I have dug a bit deeper and found that it comes from https://github.com/read-write-web/rww-play/blob/dev/app/rww/play/PlayWriterBuilder.scala#L67-73:

  /**
   * turn a blocking writer into an enumerator
   * todo: perhaps save some memory by not doing this operation in one chunk
   * todo: the base url
   *
   * @param writer
   * @tparam Obj
   * @return
   */
  def toEnum[Obj](writer: Writer[Obj,Try,_]) =
    (obj: Obj) => {
      utils.FileUtils.using(new ByteArrayOutputStream()) { out =>
        val tw = writer.write(obj, out, "http://localhost:8888/") // TODO ???????
        Enumerator(out.toByteArray)
      }
    }

I have then tried to figure out why, when requesting an n-triples resource through CORS proxy I always get the correct base URL and when I request the same resource normally, the default base URL (http://localhost:8888/) is used. What I have found is that CORS proxy is using absolute graph to write responses and when we request a resource normally, relative graph is used.

https://github.com/read-write-web/rww-play/blob/dev/app/rww/play/CORSProxy.scala#L118-129 :

   private def createResultForLDPR(ldpr: LDPR[Rdf], writer: Writer[Rdf#Graph, Try, Any])
                                 (implicit request: PlayRequest[AnyContent]): Result = {
    // TODO maybe it's not a good idea to put AllowCredentialsHeader here? needs to be checked
    val hdrs = request.headers.toSimpleMap - "ContentType" + AllowCredentialsHeader
    //todo: this needs to be refined a lot, and thought through quite a lot more carefully
    val corsHeaders = if (!hdrs.contains("Access-Control-Allow-Origin")) {
      hdrs + AllowOriginHeader(request)
    } else {
      hdrs
    }
    //HERE
    result(203, writer)(ldpr.graph).withHeaders(corsHeaders.toSeq: _*)
  }

https://github.com/read-write-web/rww-play/blob/dev/app/controllers/ldp/ReadWriteWebControllerGeneric.scala#L194-226 :

   private def writeGetResult(authResult: AuthResult[NamedResource[Rdf]])
                            (implicit request: PlayApi.mvc.Request[AnyContent]): Result = {
    def commonHeaders: List[(String, String)] =
      allowHeaders(authResult) :::
        etagHeader(authResult) :::
        updatedHeader(authResult) :::
        userHeader(authResult.authInfo.user).toList

    authResult.result match {
      case ldpr: LDPR[Rdf] => {
        writerFor[Rdf#Graph](request).map { wr =>
          val headers =
            "Access-Control-Allow-Origin" -> "*" ::
              "Accept-Patch" -> Syntax.SparqlUpdate.mimeTypes.head.mime :: //todo: something that is more flexible
              linkHeaders(ldpr) ::
              commonHeaders
          //HERE
          result(200, wr, Map(headers: _*))(ldpr.relativeGraph)
        } getOrElse {
          play.api.mvc.Results.UnsupportedMediaType("could not find serialiser for Accept types " +
            request.headers.get(play.api.http.HeaderNames.ACCEPT))
        }
      }
      case bin: BinaryResource[Rdf] => {
        val contentType = bin.mime.mime
        val headers =  "Content-Type" -> contentType :: linkHeaders(bin)::commonHeaders

        Result(
          header = ResponseHeader(200, Map(headers:_*)),
          body = bin.readerEnumerator(1024 * 8)
        )
      }
    }
  }

My proposed solution is to use absolute graph in normal mode as well. I have tried it and it works.

bblfish commented 9 years ago

ok, yes. Relative graphs should be used rarely.