Don't set X-XSRF-TOKEN when the user isn't logged in

[nkeor]

An HTTP header cannot be empty, and although some webservers allow this (nginx, Apache), others answer 400 Bad Request (lighttpd), preventing the widget from loading.

I'm not sure this is the most elegant way to do it, that's why this is marked as a draft.

[gstrauss]

Cross-reference: opnsense/core#4917

The next version of lighttpd (lighttpd 1.4.60, not yet released) will relax this unintentional restriction which surfaced in the lighttpd HTTP/2 implementation.

(It is still poor behavior for a client to send an HTTP request header with an empty value.)

[nkeor]

Oh, nice! I hadn't found anything about that.

This bug had me banging my head to the desk for two days 😆
Shall we still remove the X-XSRF-TOKEN header when it's empty?

EDIT: I think yes, but maybe I can just wait until the next lighttpd update. Right now I'm proxying the requests to /web/ to a custom built frontend with this patch.

[gstrauss]

I agree that it is a good idea to skip sending an HTTP request header with an empty value.

[paskal]

Does make sense to me.

[umputun]

Generally speaking, this is not a bug on remark42 side, but rather some deficiency of the proxy (Lighttpd). I don't have anything against not sending a no-value header but either way I see no reason why such a header may break anything.

Take a look at another project reproxy by the same authors, this one won't do insane things like Lighttpd does ;)

[akellbl4]

@gstrauss I don't think it's poor because if it's supported by browsers it should be supported by http servers.

But I agree with the PR. If we don't need to send some headers we shouldn't.

[nkeor]

@akellbl4: Actually, with the previous behavior (empty header), if I use Firefox to re-send the request to the server (the one Lighttpd answered 400), Firefox removes the empty header.

@umputun: reproxy looks like a nice piece of software. I'll look into it!
But as @gstrauss said, this behavior will be fixed in Lighttpd 1.4.60.

[akellbl4]

if I use Firefox to re-send the request to the server (the one Lighttpd answered 400), Firefox removes the empty header.

It's problem of Lighttpd :) As you can see Firefox accepts sending empty header.

[nkeor]

if I use Firefox to re-send the request to the server (the one Lighttpd answered 400), Firefox removes the empty header.

It's problem of Lighttpd :) As you can see Firefox accepts sending empty header.

Because it's being sent from a JS file. But if it's Firefox making the request, it removes it.
Another example:

$ curl -H "X-XSRF-TOKEN: " https://remark42.example.org/api/v1/config\?site=example

Curl doesn't send the header.
But as gstrauss said above, Lighttpd 1.4.60 will match Apache and nginx's behavior: ignore the header.

[codecov]

Codecov Report

Merging #1043 (927bca1) into master (91deae5) will decrease coverage by 0.10%.
The diff coverage is 18.18%.

❗ Current head 927bca1 differs from pull request most recent head f8d0807. Consider uploading reports for the commit f8d0807 to get more accurate results

@@            Coverage Diff             @@
##           master    #1043      +/-   ##
==========================================
- Coverage   44.28%   44.18%   -0.11%     
==========================================
  Files         126      126              
  Lines        2897     2906       +9     
  Branches      653      653              
==========================================
+ Hits         1283     1284       +1     
- Misses       1602     1610       +8     
  Partials       12       12              

Impacted Files	Coverage Δ
frontend/app/components/root/root.tsx	0.00% <0.00%> (ø)
frontend/app/common/fetcher.ts	94.82% <66.66%> (-1.61%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea15b28...f8d0807. Read the comment docs.

[paskal]

Should we remove XSRF_HEADER from the import where the linter detected unused variable (frontend/app/common/fetcher.test.ts)?

[nkeor]

@paskal, fixed. Also fixed prettier's complain about the indentation.

[umputun]

thx