harwey / cups4j

Cups4j Java printing library for CUPS/IPP
http://cups4j.org
GNU Lesser General Public License v3.0
130 stars 64 forks source link

Create host header from CupsClient configuration #41

Closed jordi-farre closed 3 years ago

jordi-farre commented 3 years ago

Hi!

For the current version, to work with a CUPS server that it's not in localhost, you need to add a property cups.ourhostname.

If you look in this link:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host

The Host header is the hostname + port for the destination host, in this case the CUPS server.

We already have the hostname and the port configured in the CupsClient creation, so I think we don't need and additional property to configure this.

Thanks!

juliender commented 3 years ago

Thanks, I struggled on that for hours. I had a "bad request" response from CUPS when trying to getPrinter remotely (was ok locally)

jordi-farre commented 3 years ago

Thanks, I struggled on that for hours. I had a "bad request" response from CUPS when trying to getPrinter remotely (was ok locally)

Thanks, @juliender. In my opinion that property is a bit hidden in the code and I lost some time trying to figure out the problem. Hope @harwey find my change useful in order to avoid using that property

gmuth commented 3 years ago

I have not checked this, but I would expect Apache Http Client to set the http 1.1 Host header correct by default. So I would remove the line that sets the wrong host header completely. While your solution probably works fine it most likely restores just the correct value that apache http client would use anyway.

jordi-farre commented 3 years ago

Hi, @gmuth I think you are right. In the previous version, this header was calculated by the HTTP client. I think your solution it's a better approach. I will close this PR. If you want I can create another PR removing that line. Thanks.