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

http1.1 chunked encoding #548

Closed
alacn1 opened this issue Jun 23, 2023 · 6 comments
Closed

http1.1 chunked encoding #548

alacn1 opened this issue Jun 23, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@alacn1
Copy link

alacn1 commented Jun 23, 2023

ddclient v3.10.0 is broken, failing to parse server response because chunked transfer-encoding.

SENDING:  GET /... HTTP/1.1
SENDING:  Host: ...
SENDING:  User-Agent: ddclient/3.10.0
SENDING:  Connection: close
[...]
RECEIVE:  HTTP/1.1 200 OK
[...]
RECEIVE:  Connection: close
RECEIVE:  Transfer-Encoding: chunked
RECEIVE:
RECEIVE:  15
RECEIVE:  nochg xxx.xxx.xxx.xxx
RECEIVE:  0
RECEIVE:
FAILED:   updating yyy.yyy.com: unexpected status (15)

PR #333 9a44eeb changed from HTTP/1.0 to HTTP/1.1 in geturl fetch_via_socket_io.
Chunked transfer encoding is required for HTTP/1.1.

To get it working need revert request to HTTP/1.0 or add support chunked encoding for HTTP/1.1.

@SuperSandro2000 SuperSandro2000 added the bug Something isn't working label Jul 4, 2023
@SuperSandro2000
Copy link
Member

Chunked encoding would be a new feature and those are no longer accepted in the form of issues.

@SuperSandro2000 SuperSandro2000 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 4, 2023
@straight-shoota
Copy link

straight-shoota commented Oct 16, 2023

This issue is not a feature request for supporting chunked encoding.
It's bug report about a regression introduced in 3.10 with the incomplete update to HTTP/1.1.

I have encountered exactly the same issue: The server responds with a chunked response. It is allowed to do so because ddclient's request states HTTP/1.1 as the protocol version.
But ddclient is unable to understand chunked responses. It treats the chunk length in the first line of the response as the actual payload. And cannot make any sense of it.

For my setup, the same configuration works fine with 3.9.1 but it errors in 3.10.0.

Compare the HTTP logs:

v3.10.0

SENDING:  GET /nic/update?system=dyndns&hostname=example.com&myip=x.x.x.x HTTP/1.1
SENDING:  Host: ddns.do.de
SENDING:  Authorization: Basic XXX
SENDING:  User-Agent: ddclient/3.10.0
SENDING:  Connection: close
SENDING:
RECEIVE:  HTTP/1.1 200 OK
RECEIVE:  Server: nginx
RECEIVE:  Date: Mon, 16 Oct 2023 15:51:48 GMT
RECEIVE:  Content-Type: text/html; charset=UTF-8
RECEIVE:  Transfer-Encoding: chunked
RECEIVE:  Connection: close
RECEIVE:  Vary: Accept-Encoding
RECEIVE:  Expires: Thu, 19 Nov 1981 08:52:00 GMT
RECEIVE:  Cache-Control: no-store, no-cache, must-revalidate
RECEIVE:  Pragma: no-cache
RECEIVE:  Set-Cookie: coreSID=XXX; path=/; secure; HttpOnly
RECEIVE:
RECEIVE:  14
RECEIVE:  nochg x.x.x.x
RECEIVE:  0
RECEIVE:

3.9.1

SENDING:  GET /nic/update?system=dyndns&hostname=jowemue.de&myip=x.x.x.x HTTP/1.0
SENDING:   Host: ddns.do.de
SENDING:   Authorization: Basic XXX
SENDING:   User-Agent: ddclient/3.9.1
SENDING:   Connection: close
SENDING:
SENDING:
RECEIVE:  HTTP/1.1 200 OK
RECEIVE:  Server: nginx
RECEIVE:  Date: Mon, 16 Oct 2023 16:07:30 GMT
RECEIVE:  Content-Type: text/html; charset=UTF-8
RECEIVE:  Connection: close
RECEIVE:  Vary: Accept-Encoding
RECEIVE:  Expires: Thu, 19 Nov 1981 08:52:00 GMT
RECEIVE:  Cache-Control: no-store, no-cache, must-revalidate
RECEIVE:  Pragma: no-cache
RECEIVE:  Set-Cookie: coreSID=XXX; path=/; secure; HttpOnly
RECEIVE:
RECEIVE:  nochg x.x.x.x

As the OP states, #333 updated the HTTP request to use protocol version HTTP/1.1.
But that change is flawed: it indicates that ddclient understands HTTP/1.1. Yet apparently it does not implement the standard completely, in particular the Transfer-Encoding: chunked header seems to be unrecognized and unsupported.

The same error can be seen in many other issues but apparently it hasn't been properly identified yet: #483, #338, #347, #503, #525, #414, #354, #513, #499

#503 even received a bug fix in #506. It covers exactly this problem, only in a very special case for duckdns.

As some issues (like #513) a workaround is to use curl=yes - curl of course fully implements HTTP/1.1.

#542 seems to fix this issue (I haven't verified it myself yet).
I would suggest to consider this for a 3.10.1 bugfix release.

@SuperSandro2000
Copy link
Member

The maintainers should just ditch the hand crafted http code and either replace it with a library or always use curl.

@LenardHess
Copy link
Contributor

We've already opted to require curl as the latest prerelease (v3.11.0_1, refer to the changelog ).

@LenardHess
Copy link
Contributor

@straight-shoota I expect this to work since curl is now always used - but it never hurts to confirm. Can you verify that v3.11.0_1 no longer has the issue?

@straight-shoota
Copy link

Yes, I can confirm this issue does not reproduce with v3.11.0_1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants