Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ETAG being a trailer prevents concurrent config updates from browsers #5619

Closed
anderspitman opened this issue Jul 6, 2023 · 4 comments
Closed
Labels
deferred ⏰ We'll come back to this later

Comments

@anderspitman
Copy link

I'm working on a dashboard app that includes some Caddy configuration functionality. I'm accessing the Caddy admin endpoint directly from the browser (it's protected with authentication). Unfortunately trailer headers are apparently not usable from fetch/XHR (would love to be wrong about this). See for example: whatwg/fetch#34

Does the ETAG really need to be a trailer? I haven't checked the code but I'm assuming this is the case because the config is streamed? Are configs generally large enough/modified often enough that streaming improves performance?

@mholt
Copy link
Member

mholt commented Jul 6, 2023

That sounds like a cool project! I'd love to see it when you have it working :)

Unfortunately trailer headers are apparently not usable from fetch/XHR (would love to be wrong about this)

Are we sure that's still the case? This table at MDN says all browsers support reading Trailers: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Trailer#browser_compatibility

... Oh, but this 1+ year-old open issue with no response says the table is wrong:

mdn/browser-compat-data#14703

Dang. 🤔

Does the ETAG really need to be a trailer?

Strictly, no.

I haven't checked the code but I'm assuming this is the case because the config is streamed?

Basically, yes.

In our experience, the larger the configurations are more likely to be managed more frequently, and hence have more need of the Etag; but if we compute the Etag for inclusion in the headers, we'd have to buffer the whole config. And since these configs are often large, it could use significantly more memory to buffer all that.

@anderspitman
Copy link
Author

anderspitman commented Jul 6, 2023

That makes sense. In my particular project, I'm also providing access to a docker socket to the browser 👀. Since this represents a large attack surface should someone compromise the web authentication, eventually I'll probably implement an additional proxy layer that locks things down as much as possible. That would be a natural place to do something similar with Caddy, including buffering/forwarding the ETAG or just handling concurrency myself.

That sounds like a cool project! I'd love to see it when you have it working :)

👍 It's a project to make selfhosting more accessible to nontechnical people. Think caddy+wireguard running in docker on windows using OAuth to establish a tunnel and a nice web UI for installing and managing apps as separate docker containers. No CLI or networking knowledge required. Aiming for a beta launch in the next couple months.

@mholt
Copy link
Member

mholt commented Jul 6, 2023

Wow, that's really cool!! I can't wait to see it. Honestly, if I had more time, it sounds like something I'd build.

That would be a natural place to do something similar with Caddy, including buffering/forwarding the ETAG or just handling concurrency myself.

True. But I can see why that's annoying.

I'd like to avoid buffering this response if possible; maybe we can revisit this later on? I think browsers should implement the spec, frankly.

I don't want to completely stall your project though. Maybe as a workaround in the meantime, allow only 1 concurrent config editor at a time.


PS. Did I know you are in SLC? 🤔 We could meet up over lunch sometime and discuss the project if you want to.

@mholt mholt added the deferred ⏰ We'll come back to this later label Jul 6, 2023
@mholt mholt closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2023
@anderspitman
Copy link
Author

Wow, that's really cool!! I can't wait to see it. Honestly, if I had more time, it sounds like something I'd build.

It's been quite the rabbit hole. Coming up on 4 years, and I think I'm finally almost back out of it having built what I set out to.

I'd like to avoid buffering this response if possible; maybe we can revisit this later on? I think browsers should implement the spec, frankly.

I agree. Doesn't seem like it would be that hard. IIRC it's the main thing missing for native browser support for GRPC.

I don't want to completely stall your project though. Maybe as a workaround in the meantime, allow only 1 concurrent config editor at a time.

Since I anticipate most deployments will only be managed by a single user, it's more of an academic concern than a practical one. Eventually I'll need to solve it but I don't anticipate anytime soon.

PS. Did I know you are in SLC? thinking We could meet up over lunch sometime and discuss the project if you want to.

We've never met, but I've long appreciated your work. certmagic in particular has served me well. Totally down to grab lunch. Contact info at https://apitman.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deferred ⏰ We'll come back to this later
Projects
None yet
Development

No branches or pull requests

2 participants