nodeSolidServer / node-solid-server

Solid server on top of the file-system in NodeJS
https://solidproject.org/for-developers/pod-server
Other
1.78k stars 303 forks source link

HEAD doesn't set right Content-Type or right Content-Length headers #454

Open elf-pavlik opened 8 years ago

elf-pavlik commented 8 years ago

for example GET request to foo.png includes correct header Content-Type: image/png but HEAD request to foo.png responds with incorrect header Content-Type: text/plain; charset=utf-8

dmitrizagidulin commented 7 years ago

Interesting.. thanks for reporting this!

dmitrizagidulin commented 7 years ago

Also address setting the correct Content-Length header on HEAD requests.

timbl commented 7 years ago

About time this serious breakage of the HTTP protocol was fixed

timbl commented 6 years ago

Grumble. The bad content-length makes checking that JS library files are the right ones more difficult. It is basic HTTP that headers on HEAD MUST be exactly the ones you would get with GET timbl added the priority label on Feb 21, 2017

RubenVerborgh commented 6 years ago

Might be fixed with #662.

megoth commented 6 years ago

@elf-pavlik hey, I've been having trouble reproducing the bug with Content-Type; could you maybe send me the PNG file you had trouble with?

The bug with Content-Length I've been able to reproduce though (as there is no property with that), so I'll continue working on that for now ^_^

megoth commented 6 years ago

@kjetilk and I are realizing that this might not be best handled in the current code; Content-Length should be handled automatically as part of http-handling in general, and there is probably some middleware out there that we could use.

We're suggesting that this get labelled with “test suite” (or something like it), indicating that we will write tests that handles the various use cases where Content-Length should be set, and that this test-suite will be used by us and others wanting to implement a Solid-compliant server.

We're also suggesting that this issue be labelled with revisit, as this issue is not critical.

megoth commented 6 years ago

If people agree with the above I'll also remove the label priority.

RubenVerborgh commented 6 years ago

@megoth It was a priority for @timbl.

kjetilk commented 6 years ago

So, this is really two issues, Content-Type, which is a really nasty bug, but something @megoth has not been able to reproduce, and secondly Content-Length, which should be implemented by a standalone module in the architecture. I think we should split this into two issues.

kjetilk commented 6 years ago

Hmmm, or it should be a bug on that HEAD isn't identical to GET headers, which is also just really weird that the framework doesn't do right out of the box.

megoth commented 6 years ago

Yes, once again I didn't quite get the task =P I've been able to reproduce the bug now, and will continue on it.

RubenVerborgh commented 6 years ago

If you ask me, we can just have the exact same logic, except we don't write a body (so catch all res.end/write calls or pipes to it). That should do for now; optimizations always possible later.

kjetilk commented 6 years ago

I'm just baffled this is actually an issue at all... This should be part of the HTTP stack of the server environment, and not the concern of developers. Like it is in the Plack environment, e.g.: https://github.com/kjetilk/RDF-LinkedData/blob/master/script/linked_data.psgi#L52

RubenVerborgh commented 6 years ago

This should be part of the HTTP stack of the server environment

Yes, but we're playing server on a very low level here. I think it is good that the Solid server takes ownership of the HTTP methods; node-solid-server does not do so with the right architecture unfortunately.

kjetilk commented 6 years ago

sure, but a reasonable architecture does it in middleware, so that the server behaves HTTP compliant by default, you'd actually have to break it on purpose to break it... And you should only have to modify the behavior that needs to adapted, not all the other things.

elf-pavlik commented 6 years ago

I only reported (two years ago) the issue on Content-Type which afterwards also was reported in #632 and #789 I think one of the maintainers added the Content-Length part here.

elf-pavlik commented 6 years ago

So, this is really two issues, Content-Type, which is a really nasty bug, but something @megoth has not been able to reproduce,

I think easiest way to reproduce based on #789

HEAD curl -I https://rubentest.solidtest.space/profile/card | grep Content-Type

Content-Type: text/plain; charset=utf-8

vs

GET curl -i https://rubentest.solidtest.space/profile/card | grep Content-Type

Content-Type: text/turtle

michielbdejong commented 5 years ago

@gklyne is still seeing this issue

gklyne commented 5 years ago

Here's a simple example of what I'm seeing:

$ curl -i -H "Accept: text/turtle" -X HEAD https://localhost:8443/public/ | grep -i "content-type"
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     2    0     0    0     0      0      0 --:--:--  0:00:04 --:--:--     0curl: (56) SSLRead() return error -9806
Content-Type: application/octet-stream; charset=utf-8

Note also the curl: (56) SSLRead() return error -9806 - I don't know if that's relevant. For comparison, the same with GET:

$ curl -i -H "Accept: text/turtle" -X GET https://localhost:8443/public/ | grep -i "content-type"
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   778    0   778    0     0  11502      0 --:--:-- --:--:-- --:--:-- 11611
Content-Type: text/turtle

Also, no SSLRead error in this case.

Server console log shows:

Solid server (5.0.0-beta.7) running on https://localhost:8443/

and for HEAD:

  solid:get /public/ on localhost +0ms
  solid:get HEAD only +0ms

and for GET:

  solid:get /public/ on localhost +0ms
  solid:handlers GET -- Reading directory +2ms
  solid:handlers Files in directory: .acl,dcd0fc80-test_workset,test_workset +0ms

(I've elided a lot of solid:ACL tracing.)

CxRes commented 4 years ago

I am still seeing this issue in April 2020.

The header in response to HEAD to a container is like this:

{
   {
    'x-powered-by': [ 'solid-server/5.2.3' ],
    vary: [ 'Accept, Authorization, Origin' ],
    'access-control-allow-credentials': [ 'true' ],
    'access-control-expose-headers': [
      'Authorization, User, Location, Link, Vary, Last-Modified, ETag, Accept-Patch, Accept-Post, Updates-Via, Allow, WAC-Allow, Content-Length, WWW-Authenticate, MS-Author-Via'
    ],
    allow: [ 'OPTIONS, HEAD, GET, PATCH, POST, PUT, DELETE' ],
    link: [
      '<.acl>; rel="acl", <.meta>; rel="describedBy", <http://www.w3.org/ns/ldp#Container>; rel="type", <http://www.w3.org/ns/ldp#BasicContainer>; rel="type"'
    ],
    'wac-allow': [ 'user="read write append control",public="read"' ],
    'ms-author-via': [ 'SPARQL' ],
    'updates-via': [ 'wss://cxres.inrupt.net' ],
    'content-type': [ 'application/octet-stream; charset=utf-8' ],
    'content-length': [ '2' ],
    etag: [ 'W/"2-nOO9QiTIwXgNtWtBJezz8kv3SLc"' ],
    'set-cookie': [
      'nssidp.sid=s%3A58cbIKn7xKh-xZqNUfR01Ehu0pesWA5T.5gxFVpLe5yr0zpyD4%2BF%2BjvLoDulc9UDbRxmbH5pIcIg; Domain=.inrupt.net; Path=/; Expires=Wed, 08 Apr 2020 19:50:15 GMT; HttpOnly; Secure'
    ],
    date: [ 'Tue, 07 Apr 2020 19:50:15 GMT' ],
    connection: [ 'close' ]
  }
} 

And in response to a GET, headers to the same container look like this:

{
  {
    'x-powered-by': [ 'solid-server/5.2.3' ],
    vary: [ 'Accept, Authorization, Origin' ],
    'access-control-allow-credentials': [ 'true' ],
    'access-control-expose-headers': [
      'Authorization, User, Location, Link, Vary, Last-Modified, ETag, Accept-Patch, Accept-Post, Updates-Via, Allow, WAC-Allow, Content-Length, WWW-Authenticate, MS-Author-Via'
    ],
    allow: [ 'OPTIONS, HEAD, GET, PATCH, POST, PUT, DELETE' ],
    link: [
      '<.acl>; rel="acl", <.meta>; rel="describedBy", <http://www.w3.org/ns/ldp#Container>; rel="type", <http://www.w3.org/ns/ldp#BasicContainer>; rel="type"'
    ],
    'wac-allow': [ 'user="read write append control",public="read"' ],
    'ms-author-via': [ 'SPARQL' ],
    'updates-via': [ 'wss://cxres.inrupt.net' ],
    'content-type': [ 'text/turtle' ],
    'set-cookie': [  'nssidp.sid=s%3Az1UnmegrbvAcfj4V2wurV8dCI7gfoylH.km%2FIzSslorI5xhaA8uJfCiiTk3q%2Fv6mWcY2lDud%2Blw8; Domain=.inrupt.net; Path=/; Expires=Wed, 08 Apr 2020 19:50:16 GMT; HttpOnly; Secure'
    ],
    date: [ 'Tue, 07 Apr 2020 19:50:16 GMT' ],
    connection: [ 'close' ],
    'transfer-encoding': [ 'chunked' ]
  }
}

As you can clearly see the content-type and other things are inconsistent!!!!?????.

csarven commented 2 weeks ago
$ curl -iIk https://csarven.localhost:8443/profile/card | grep -iE "content-(type|length):"
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     2    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
Content-Type: text/turtle; charset=utf-8
Content-Length: 2

$ curl -ik https://csarven.localhost:8443/profile/card | grep -iE "content-(type|length):"
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1612    0  1612    0     0  34419      0 --:--:-- --:--:-- --:--:-- 35043
Content-Type: text/turtle

Seems charset and Content-Length are missing for GET.

Perhaps the following may some insight:

$ curl -ikH'Accept: text/html' https://csarven.localhost:8443/profile/card | grep -iE "content-(type|length):"
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   518  100   518    0     0  10941      0 --:--:-- --:--:-- --:--:-- 11021
Content-Type: text/html; charset=utf-8
Content-Length: 518

$ curl -iIkH'Accept: text/html' https://csarven.localhost:8443/profile/card | grep -iE "content-(type|length):"
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     2    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
Content-Type: text/turtle; charset=utf-8
Content-Length: 2

(This is besides the fact that NSS awkwardly generates an HTML representation for "SolidOS Web App", which has nothing to do with even being vaguely equivalent to a WebID Profile Document!)

Seems to produce charset for text/html media type.