Re: [fossil-users] bug : /zip or /tar error page response return HTTP/1.1 200 OK

[Ross Berteig]

On 6/9/2017 1:26 PM, Warren Young wrote:

On Jun 9, 2017, at 7:11 AM, Richard Hipp  wrote:

Perhaps /tarball and /zip are special in the sense that they are often
accessed by scripts.

I don’t have any data on whether that is true, but you are right that /webpages 
frequently accessed by scripts should return sensible HTTP result codes.

/webpages exclusively accessed from a browser can return 200 in all cases, 
since the only way to see the error code is to drop into the browser’s 
developer tools.  If the error report is inline in the HTML, 200 is even 
arguably correct.


A 404 page is almost always valid HTML, and often cleverly styled. Most 
browsers will show it and other status pages to the end user. But you 
are right that they won't show the actual HTTP status, the end user is 
trusting that the error page is presenting sensible information.


I suppose you could argue that /zip (and /tarball) should not present 
text, ever.  But in their normal usage by scripts, the returned content 
would be completely ignored unless status was 200.



The JSON API should be scrupulous about returning correct HTTP result codes.


The JSON API returns 200 for any result that can be best explained by a 
JSON result record, which itself contains a status. This is consistent 
with treating HTTP as the transport protocol. The HTTP transaction 
worked, so 200 is the correct response code, even if a parameter made 
the /json API endpoint not take the intended action.


A code other than 200 would be correct if /json were not a valid URL at 
all, or if some other hard failure occurred. Other than JSON not 
compiled in, there aren't supposed to be very many cases where the JSON 
engine can't describe the situation for itself.


Look at it this way. If access to /json/nosuchapi returns HTTP status 
200, then the caller can expect to have a JSON document to parse. If it 
doesn't, then it can't expect the result contains anything parseable at 
all, and in general the content of a 404 page is human readable text.


Given that the /json namespace is intended for use by automation that 
wants to consume JSON, attempting to guarantee that any access within 
that namespace will be HTTP 200 with a JSON packet explaining the API 
call results seems reasonable.


--

Ross Berteig   r...@cheshireeng.com
Cheshire Engineering Corp.   http://www.CheshireEng.com/
+1 626 303 1602

___
fossil-users mailing list
fossil-users@lists.fossil-scm.org
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users

Re: [fossil-users] bug : /zip or /tar error page response return HTTP/1.1 200 OK

[Warren Young]

On Jun 9, 2017, at 7:11 AM, Richard Hipp  wrote:
> 
> Perhaps /tarball and /zip are special in the sense that they are often
> accessed by scripts.

I don’t have any data on whether that is true, but you are right that /webpages 
frequently accessed by scripts should return sensible HTTP result codes.

/webpages exclusively accessed from a browser can return 200 in all cases, 
since the only way to see the error code is to drop into the browser’s 
developer tools.  If the error report is inline in the HTML, 200 is even 
arguably correct.

The JSON API should be scrupulous about returning correct HTTP result codes.
___
fossil-users mailing list
fossil-users@lists.fossil-scm.org
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users

Re: [fossil-users] bug : /zip or /tar error page response return HTTP/1.1 200 OK

[Richard Hipp]

On 6/9/17, Stephan Beal  wrote:
>
> Perhaps name_to_typed_rid() should set it to 404 IF running in [non-json]
> http mode? (For JSON, the API currently returns 200 everywhere and uses its
> own error codes to report what went wrong. That behaviour is arguable but
> consistent.)

Perhaps /tarball and /zip are special in the sense that they are often
accessed by scripts.  Probably webpages that deliver content for
rendering on browser screens do not care as much about the status
code.

Anyhow, I did patch of /tarball and /zip.  See check-in
https://www.fossil-scm.org/fossil/info/a10fc448ed3fce51

-- 
D. Richard Hipp
d...@sqlite.org
___
fossil-users mailing list
fossil-users@lists.fossil-scm.org
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users

Re: [fossil-users] bug : /zip or /tar error page response return HTTP/1.1 200 OK

[Stephan Beal]

On Fri, Jun 9, 2017 at 2:01 PM, Richard Hipp  wrote:

> I think there are many other examples where Fossil finds incorrect
> query parameters and reports an error in text, but still returns "200
> ok".  If returning a non-200 result codes on an error is important,
> then we have a lot of clean-up to do.
>
> Also, I'm not sure setting the result code to an error before a
> function that potentially errors-out, then resetting it back to 200
> afterwards, is the correct fix.
>

Perhaps name_to_typed_rid() should set it to 404 IF running in [non-json]
http mode? (For JSON, the API currently returns 200 everywhere and uses its
own error codes to report what went wrong. That behaviour is arguable but
consistent.)

-- 
- stephan beal
http://wanderinghorse.net/home/stephan/
"Freedom is sloppy. But since tyranny's the only guaranteed byproduct of
those who insist on a perfect world, freedom will have to do." -- Bigby Wolf
___
fossil-users mailing list
fossil-users@lists.fossil-scm.org
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users

Re: [fossil-users] bug : /zip or /tar error page response return HTTP/1.1 200 OK

[Richard Hipp]

I think there are many other examples where Fossil finds incorrect
query parameters and reports an error in text, but still returns "200
ok".  If returning a non-200 result codes on an error is important,
then we have a lot of clean-up to do.

Also, I'm not sure setting the result code to an error before a
function that potentially errors-out, then resetting it back to 200
afterwards, is the correct fix.

On 6/9/17, kowlsd3pw...@yahoo.co.jp  wrote:
> /zip/?r=error-tag-name
> this error page return HTTP/1.1 200 OK.
>
> src/zip.c : baseline_zip_page function
>
> https://www.fossil-scm.org/index.html/artifact?ln=570=12c010669a95d634
> src/tar.c : tarball_page function
>
> https://www.fossil-scm.org/index.html/artifact?ln=714=17b09ed44d15326d
>
> Since name_to_typed_rid calls fossil_fatal, it needs to call cgi_set_status
> before that.
>
> +  cgi_set_status(404, "Not Found");
>   rid = name_to_typed_rid(nRid?zRid:zName, "ci");
>   if..
>   ..
>   }
> + cgi_set_status(200, "OK");


-- 
D. Richard Hipp
d...@sqlite.org
___
fossil-users mailing list
fossil-users@lists.fossil-scm.org
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users