Waitress 1.2.0 beta 1

[Bert JW Regeer]

Hey all,

As a new years gift, I have just released a beta version of waitress 1.2.0, hopefully with a bit of luck this will be the only beta release with minimal or no changes and 1.2.0 should follow shortly.

This release has some major changes to the proxy handling in Waitress, which will be of note for those of you that use Waitress behind a reverse proxy such as NGINX/HAProxy/Apache or others.

Waitress is now able to manipulate and change the WSGI environment to match what the proxy headers are sending, and has full support for the new Forwarded header.

Documentation is available here: https://docs.pylonsproject.org/projects/waitress/en/latest/reverse-proxy.html

Please note that in the future Waitress is going to be more secure by default, and will strip known proxy headers before forwarding them on in the WSGI environ to help protect WSGI applications from accidentally using attacker provider proxy headers and their values.

Waitress will warn if you don't explicitly opt-in to the following:

If trusted_proxy is set, trusted_proxy_headers should be set to the values you explicitly want Waitress to use (and are known valid from an upstream proxy), and clear_untrusted_proxy_headers should be set to either True or False. See https://docs.pylonsproject.org/projects/waitress/en/latest/arguments.html for more information on what these knobs are and valid values.

I am hoping to get some feedback from testing, if you can, deploy this and remove any middleware you may have that is manipulating the environ and test if waitress matches expectations.

https://pypi.org/project/waitress/1.2.0b1/

pip install waitress=1.2.0b1

Thank you,

Bert JW Regeer

[Lele Gaifax]

Hi,

Bert JW Regeer <xist...@0x58.com> writes:

> I am hoping to get some feedback from testing, if you can

I installed waitress 1.2.0b1 in an application I'm developing, and the only
"suspicious" thing I noticed is the following warning log, that popups quite
often:

waitress: application-written content was ignored due to HTTP response that
may not contain a message-body: (200 OK)

Putting a debugger in task.py::Task.write(), I see the following backtrace

/usr/local/lib/python3.6/threading.py(884)_bootstrap()
-> self._bootstrap_inner()
/usr/local/lib/python3.6/threading.py(916)_bootstrap_inner()
-> self.run()
/usr/local/lib/python3.6/threading.py(864)run()
-> self._target(*self._args, **self._kwargs)
/usr/local/lib/python3.6/site-packages/waitress/task.py(78)handler_thread()
-> task.service()
/usr/local/lib/python3.6/site-packages/waitress/channel.py(336)service()
-> task.service()
/usr/local/lib/python3.6/site-packages/waitress/task.py(175)service()
-> self.execute()
/usr/local/lib/python3.6/site-packages/waitress/task.py(469)execute()
-> self.write(b'') # generate headers
> /usr/local/lib/python3.6/site-packages/waitress/task.py(353)write()
-> self.logger.warning(

Is it expected?

Thank you,
ciao, lele.
--
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
le...@metapensiero.it | -- Fortunato Depero, 1929.

[Michael Merickel]

I suspect you can ignore the warning and that it is a bug.

It seems likely that this [1] should be "if data and not self.logged_write_no_body" to avoid warning when the body is missing on a response that may contain a body?

[1] https://github.com/Pylons/waitress/blob/01ae68065f46b7b04950255882f129d770a9645b/waitress/task.py#L351

--
You received this message because you are subscribed to the Google Groups "pylons-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pylons-discus...@googlegroups.com.
To post to this group, send email to pylons-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/pylons-discuss/8736qbavgg.fsf%40metapensiero.it.
For more options, visit https://groups.google.com/d/optout.

[Bert JW Regeer]

That looks to be correct. I'll see if I can get a new beta out with that fix later tonight. I may change up how the if/else statements there to make it more clear what is going on.

Bert

To view this discussion on the web visit https://groups.google.com/d/msgid/pylons-discuss/CAKdhhwF9YMW5dKZLeCz%3DLx4mR_wGvj2_AZBeXbtMgxGAhOxs0g%40mail.gmail.com.

[Bert JW Regeer]

It was a simple change, uploaded a new version.

https://pypi.org/project/waitress/1.2.0b2/

pip install waitress==1.2.0b2

Thanks,

Bert

To view this discussion on the web visit https://groups.google.com/d/msgid/pylons-discuss/52FD77F3-8DA3-4E7D-8CED-60BBC34F3C5D%400x58.com.

[Lele Gaifax]

Bert JW Regeer <xist...@0x58.com> writes:

> It was a simple change, uploaded a new version.
>

> https://pypi.org/project/waitress/1.2.0b2/ <https://pypi.org/project/waitress/1.2.0b2/>
>
> pip install waitress==1.2.0b2

Thank you Bert!

Sorry to bother you again, but with 1.2.0b2 (I did not notice it with b1, but
it seems unlikely it is related to b2 changes) I see the following warning now
and then, always with depth=1:

waitress.queue: Task queue depth is 1

Will try to get a backtrace if needed.

[Bert JW Regeer]

That warning means you don't have enough threads to handle your incoming requests and you have connections waiting to get a response from your WSGI server.

I wonder if we need better heuristics or some tuning for it, right now it starts logging warnings if there is 1 item in the queue waiting to be processed.

You can disable logging of that warning by disabling logging for waitress.queue, using the standard Python logging setup.

Bert

> --
> You received this message because you are subscribed to the Google Groups "pylons-discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to pylons-discus...@googlegroups.com.
> To post to this group, send email to pylons-...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/pylons-discuss/87sgyadpi6.fsf%40metapensiero.it.

[Jonathan Vanasco]

On Monday, December 31, 2018 at 4:12:10 PM UTC-5, Bert JW Regeer wrote:

Please note that in the future Waitress is going to be more secure by default, and will strip known proxy headers before forwarding them on in the WSGI environ to help protect WSGI applications from accidentally using attacker provider proxy headers and their values.

It would be nice if that functionality could be implemented as middleware when not running the waitress server.

[Jonathan Vanasco]

On Monday, January 7, 2019 at 5:00:43 PM UTC-5, Jonathan Vanasco wrote:

It would be nice if that functionality could be implemented as middleware when not running the waitress server.

This release broke our dev systems from the wsgi manipulation. The issue was this line to the nginx config from the instructions:

    proxy_set_header X-Forwarded-Host $host:$server_port;

While the the `:$server_port` bit appears in many tutorials and docs on some projects, it isn't part of a standard or a standard/requirement.  Even nginx's official docs has conflicting usage patterns with it:

* https://www.nginx.com/resources/wiki/start/topics/examples/likeapache/
* https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/

I can't tell if that bit was required by your code. It doesn't look like it is required (via `task.py` lines 531+) but perhaps a warning about that would help. 

Sidenote: it looks like the strip on line 680 may be better placed just above the immediate conditional

-        if forwarded_host:
-            
-            forwarded_host = forwarded_host.strip()

# might be better as...

+        forwarded_host = forwarded_host.strip()
+        if forwarded_host:

My initial feeling on this release is that I dislike this implementation.  This approach will create a lot of added complexity for those who use waitress for Pyramid in development BUT run other servers in Production and/or Staging - we now have to deal with a deploying something where Waitress inherently takes control of this portion of wsgi manipulation in one environment, but is not run at all in other environments.  

IMHO, it would be really nice if...

1. this happened within Pyramid (not likely to happen, I know)

or
2. this functionality were exposed as a callable, so other deployments can invoke it. This will still create some pain in maintaining dual deployment logics, but invoking waitress's logic would ensure parity.

Finally...

for the docs... if using an ini file, the syntax is;

[server:main]

use = egg:waitress#main

host = 127.0.0.1

port = 5020

trusted_proxy = 127.0.0.1
trusted_proxy_headers = x-forwarded-for x-forwarded-host x-forwarded-proto x-forwarded-port

using a quoted format from the current docs will generate an error:

trusted_proxy_headers = "x-forwarded-for x-forwarded-host x-forwarded-proto x-forwarded-port"

[Bert JW Regeer]

On Jan 7, 2019, at 16:29, Jonathan Vanasco <jona...@findmeon.com> wrote:

On Monday, January 7, 2019 at 5:00:43 PM UTC-5, Jonathan Vanasco wrote:

It would be nice if that functionality could be implemented as middleware when not running the waitress server.

This release broke our dev systems from the wsgi manipulation. The issue was this line to the nginx config from the instructions:

    proxy_set_header X-Forwarded-Host $host:$server_port;

While the the `:$server_port` bit appears in many tutorials and docs on some projects, it isn't part of a standard or a standard/requirement.  Even nginx's official docs has conflicting usage patterns with it:

$server_port is only required if you are not using a default port, so that the `X-Forwarded-Host` contains the port number. Browsers by default will already send a host header like this:

Host: example.com:8080

If the URL is:

https://example.com:8080/

I am not sure if NGINX forwards the port with the host in the $host variable or not. I can remove it from the docs.

* https://www.nginx.com/resources/wiki/start/topics/examples/likeapache/
* https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/

I can't tell if that bit was required by your code. It doesn't look like it is required (via `task.py` lines 531+) but perhaps a warning about that would help. 

Sidenote: it looks like the strip on line 680 may be better placed just above the immediate conditional

-        if forwarded_host:
-            
-            forwarded_host = forwarded_host.strip()

# might be better as...

+        forwarded_host = forwarded_host.strip()
+        if forwarded_host:

It's already been stripped way before that line ever happens. That extra strip() can be removed.

My initial feeling on this release is that I dislike this implementation.  This approach will create a lot of added complexity for those who use waitress for Pyramid in development BUT run other servers in Production and/or Staging - we now have to deal with a deploying something where Waitress inherently takes control of this portion of wsgi manipulation in one environment, but is not run at all in other environments.  

So don't use waitress for this... continue to use your WSGI middleware that you are already using, and completely ignore the existence of this functionality. I would also argue that in development you likely aren't using a reverse proxy server, but even if you are, why are you using a different WSGI server in development than what you run in production when they may have entirely different behaviours or startup requirements.

IMHO, it would be really nice if...

1. this happened within Pyramid (not likely to happen, I know)

Nope.

or
2. this functionality were exposed as a callable, so other deployments can invoke it. This will still create some pain in maintaining dual deployment logics, but invoking waitress's logic would ensure parity.

I may end up writing some middleware and putting it in WebOb, but I haven't gotten that far yet.

Also, once again, you are not required to use waitress for this functionality. You may set:

clear_untrusted_proxy_headers = False

in development so you don't receive the warning (in the future this flag will be set to True by default, and Waitress will strip proxy headers), and Waitress will happily pass through all the headers into your WSGI environment just like before, as if this functionality does not exist. 

Finally...

for the docs... if using an ini file, the syntax is;

[server:main]

use = egg:waitress#main

host = 127.0.0.1

port = 5020

trusted_proxy = 127.0.0.1
trusted_proxy_headers = x-forwarded-for x-forwarded-host x-forwarded-proto x-forwarded-port

using a quoted format from the current docs will generate an error:

trusted_proxy_headers = "x-forwarded-for x-forwarded-host x-forwarded-proto x-forwarded-port"

Waitress does not inherently support ini or any other config files. Using quotes is valid for python kwargs that you pass to waitress.serve. Since ini is not directly supported or used by waitress, I will continue to document using standard Python syntax.

--
You received this message because you are subscribed to the Google Groups "pylons-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pylons-discus...@googlegroups.com.
To post to this group, send email to pylons-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/pylons-discuss/01761583-c857-412f-80c3-117c6af4f82c%40googlegroups.com.

[Bert JW Regeer]

On Jan 7, 2019, at 17:18, Bert JW Regeer <xist...@0x58.com> wrote:

On Jan 7, 2019, at 16:29, Jonathan Vanasco <jona...@findmeon.com> wrote:

On Monday, January 7, 2019 at 5:00:43 PM UTC-5, Jonathan Vanasco wrote:

It would be nice if that functionality could be implemented as middleware when not running the waitress server.

This release broke our dev systems from the wsgi manipulation. The issue was this line to the nginx config from the instructions:

    proxy_set_header X-Forwarded-Host $host:$server_port;

While the the `:$server_port` bit appears in many tutorials and docs on some projects, it isn't part of a standard or a standard/requirement.  Even nginx's official docs has conflicting usage patterns with it:

$server_port is only required if you are not using a default port, so that the `X-Forwarded-Host` contains the port number. Browsers by default will already send a host header like this:

Host: example.com:8080

If the URL is:

https://example.com:8080/

I am not sure if NGINX forwards the port with the host in the $host variable or not. I can remove it from the docs.

According to https://stackoverflow.com/questions/15414810/whats-the-difference-of-host-and-http-host-in-nginx $host does not include the port number.

To view this discussion on the web visit https://groups.google.com/d/msgid/pylons-discuss/CC8D499E-7ADE-46DA-B9D9-DD198F2B93F9%400x58.com.