Poor HTTP support #727

New Issue

2026-10-05T04:41:11+02:00

adam commented

2026-10-05 04:41:11 +02:00

In src/Listener.cpp we have:

bool bIsHTTP = (sLine.WildCmp("GET * HTTP/1.?\r\n") || sLine.WildCmp("POST * HTTP/1.?\r\n"));

This to me implies that HTTP/1.1 is supported due to the wildcard. If we refer to RFC2616 section 9, we know that there are other HTTP methods, including:

OPTIONS - CORS pre-flights, list of supported HTTP methods
PUT - use in RESTful HTTP APIs to store information
PATCH - use in RESTful HTTP APIs to update information
DELETE - use in RESTful HTTP APIs to remove information

It should be apparent that the existing check is a problematic way of determining whether a request is HTTP or not. When attempting to use any of these methods, we get an error message:

➜  ~ curl -XPUT https://somebnc.com:<port>
ERROR :We don't take kindly to your types around here!

This prevents web modules from fully supporting HTTP client functionality, offering RESTful APIs using different verbs than GET and POST etc.

What I'd like to see:

A better check to determine if a request is HTTP or not, that doesn't use a hardcoded list of methods.
bool IsPost() const; either replaced with const CString& GetRequestMethod() const; or kept as a convenience function (best for BC) and augmented with the new GetRequestMethod() method. This would allow web modules to make use of the methods.
TRACE/OPTIONS could potentially be supported in the core, but care would need to be taken not to allow cookie session theft for TRACE in particular (regardless of HttpOnly flags!).

Feedback on these suggestions would be greatly appreciated.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/38332818-poor-http-support?utm_campaign=plugin&utm_content=tracker%2F1759&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F1759&utm_medium=issues&utm_source=github).

In `src/Listener.cpp` we have: ``` cpp bool bIsHTTP = (sLine.WildCmp("GET * HTTP/1.?\r\n") || sLine.WildCmp("POST * HTTP/1.?\r\n")); ``` This to me implies that `HTTP/1.1` is supported due to the wildcard. If we refer to RFC2616 section 9, we know that there are other HTTP methods, including: - **OPTIONS** - CORS pre-flights, list of supported HTTP methods - **PUT** - use in RESTful HTTP APIs to store information - **PATCH** - use in RESTful HTTP APIs to update information - **DELETE** - use in RESTful HTTP APIs to remove information It should be apparent that the existing check is a problematic way of determining whether a request is HTTP or not. When attempting to use any of these methods, we get an error message: ``` ➜ ~ curl -XPUT https://somebnc.com:<port> ERROR :We don't take kindly to your types around here! ``` This prevents web modules from fully supporting HTTP client functionality, offering RESTful APIs using different verbs than GET and POST etc. --- What I'd like to see: - A better check to determine if a request is HTTP or not, that doesn't use a hardcoded list of methods. - `bool IsPost() const;` either **replaced** with `const CString& GetRequestMethod() const;` or kept as a convenience function (best for BC) and augmented with the new `GetRequestMethod()` method. This would allow web modules to make use of the methods. - TRACE/OPTIONS could potentially be supported in the core, but care would need to be taken not to allow cookie session theft for TRACE in particular (regardless of `HttpOnly` flags!). Feedback on these suggestions would be greatly appreciated. ## <bountysource-plugin> --- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/38332818-poor-http-support?utm_campaign=plugin&utm_content=tracker%2F1759&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F1759&utm_medium=issues&utm_source=github). </bountysource-plugin>

adam commented

2026-10-05 04:41:12 +02:00

The suggestions are cool and all, but I don't really think the focus for ZNC should be on the web admin. You could really extend on the whole "znc should have a full web server built in with POST and GET and PUT and ...", but the core functionality for ZNC is its bouncer, which has enough problems as it is already.

I think for 90% of the users, these features wouldn't be useful (and I'd rather have a working CLI than HTTP API), and to me this would only introduce more vulnerabilities by exposing more things. It's hard to turn off the webadmin so people who don't want a fully loaded webadmin they never use is annoying as well.

The issue is valid, but I'd rather ZNC works on the bouncer side of things (which is rather sucky as is), rather than the webadmin which is mostly a front end for users to convey information better.

The suggestions are cool and all, but I don't really think the focus for ZNC should be on the web admin. You could really extend on the whole "znc should have a full web server built in with POST and GET and PUT and ...", but the core functionality for ZNC is its bouncer, which has enough problems as it is already. I think for 90% of the users, these features wouldn't be useful (and I'd rather have a working CLI than HTTP API), and to me this would only introduce more vulnerabilities by exposing more things. It's hard to turn off the webadmin so people who don't want a fully loaded webadmin they never use is annoying as well. The issue is valid, but I'd rather ZNC works on the bouncer side of things (which is rather sucky as is), rather than the webadmin which is mostly a front end for users to convey information better.

adam commented

2026-10-05 04:41:13 +02:00

The suggestions are cool and all, but I don't really think the focus for ZNC should be on the web admin.

Fair enough, people have different visions of what should change in software - however as mentioned on IRC if there was a PR for this issue then this point would really be irrelevant. webadmin is shipped with ZNC and people do use (and like using) it.

You could really extend on the whole "znc should have a full web server built in with POST and GET and PUT and ...", but the core functionality for ZNC is its bouncer, which has enough problems as it is already.

I asked you to expand on this in #znc and didn't really get a full answer - but in any case, I don't see why the issues with the bouncer are relevant to this part of the codebase. You mentioned developer focus, but I'm more of a web developer than a BNC developer and so it's really questionable as to whether I'd be able to help with anything else at all, especially in C++.

If there's a tested PR then the impact on the core team is minimal.

I think for 90% of the users, these features wouldn't be useful

I imagine >90% of users do use the webadmin module to administer ZNC and would certainly make good use of the HTTP server inside even if not directly. I don't see why opening up more possibilities to module authors and making the server more compliant with the HTTP spec is a bad idea.

There's also scope to consider different approaches for webadmin to improve it, with better HTTP support.

to me this would only introduce more vulnerabilities by exposing more things

Adding support for OPTIONS requests would allow a secure CORS policy to be set by the BNC administrator. Additionally, without modules making use of the new verbs there would presumably be nothing in the core that would necessarily use them - and thus no increased attack surface.

The worry now is that because these (very valid) requests aren't parsed properly they're treated as IRC commands instead which is much more of a risk, especially if they can be launched cross-site.

It's hard to turn off the webadmin so people who don't want a fully loaded webadmin they never use is annoying as well.

This is arguably a separate issue, and I do agree with the sentiment behind it.

The issue is valid, but I'd rather ZNC works on the bouncer side of things

Mostly negated by providing a PR, as discussed above.

> The suggestions are cool and all, but I don't really think the focus for ZNC should be on the web admin. Fair enough, people have different visions of what should change in software - however as mentioned on IRC if there was a PR for this issue then this point would really be irrelevant. `webadmin` is shipped with ZNC and people do use (and like using) it. > You could really extend on the whole "znc should have a full web server built in with POST and GET and PUT and ...", but the core functionality for ZNC is its bouncer, which has enough problems as it is already. I asked you to expand on this in `#znc` and didn't really get a full answer - but in any case, I don't see why the issues with the bouncer are relevant to this part of the codebase. You mentioned developer focus, but I'm more of a web developer than a BNC developer and so it's really questionable as to whether I'd be able to help with anything else at all, especially in C++. If there's a tested PR then the impact on the core team is minimal. > I think for 90% of the users, these features wouldn't be useful I imagine >90% of users **do** use the `webadmin` module to administer ZNC and would certainly make good use of the HTTP server inside even if not directly. I don't see why opening up more possibilities to module authors and making the server more compliant with the HTTP spec is a bad idea. There's also scope to consider different approaches for `webadmin` to improve it, with better HTTP support. > to me this would only introduce more vulnerabilities by exposing more things Adding support for **OPTIONS** requests would allow a secure CORS policy to be set by the BNC administrator. Additionally, without modules making use of the new verbs there would presumably be nothing in the core that would necessarily use them - and thus no increased attack surface. The worry _now_ is that because these (very valid) requests aren't parsed properly they're treated as IRC commands instead which is _much more of a risk_, especially if they can be launched cross-site. > It's hard to turn off the webadmin so people who don't want a fully loaded webadmin they never use is annoying as well. This is arguably a separate issue, and I do agree with the sentiment behind it. > The issue is valid, but I'd rather ZNC works on the bouncer side of things Mostly negated by providing a PR, as discussed above.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: adam/znc#727