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

Ability to disable 'Transfer-Encoding: chunked' #9988

Closed
iSerganov opened this issue May 23, 2023 · 6 comments
Closed

Ability to disable 'Transfer-Encoding: chunked' #9988

iSerganov opened this issue May 23, 2023 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@iSerganov
Copy link

Background

There are numerous legacy client devices for audio playback that doesn't support chunked Transfer Encoding.
At the same time they use HTTP/1.1 clients to request audio streams. Not supporting chunked Transfer-Encoding by HTTP/1.1 clients is a spec violation as RFC-7230 clearly says that HTTP/1.1 clients must do that. My guess is that the reason for such spec neglect is that not so long ago all Icecast/SHOUTcast streaming servers returned only HTTP/1.0 responses and never supported chunking.

Though all modern devices support chunking, this still appears to be quite a common case in streaming universe as thousands of legacy clients are still eager to access audio streams all over the world.

Fix

Actual fix to this issue is very simple: you detect a request from legacy device analyzing User-Agent HTTP header and add the following line to your code (Golang):

w.Header().Set("Transfer-Encoding", "identity")

But...

... this doesn't work if a streaming server is behind Nginx ingress controller which always returns
[http @ 0x600003d901e0] header='Transfer-Encoding: chunked' to all HTTP client requests until Content-Length header is specified. This behavior is 100% correct and spec compliant however breaks the fix from the previous paragraph.

Request

Is it possible to add control over Transfer-Encoding response header through e.g. new annotation:
nginx.ingress.kubernetes.io/disable-chunked-transfer-encoding: true?

@iSerganov iSerganov added the kind/feature Categorizes issue or PR as related to a new feature. label May 23, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels May 23, 2023
@longwuyuan
Copy link
Contributor

Do you want to submit a PR ?

@iSerganov
Copy link
Author

I think I can try!

@github-actions
Copy link

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 23, 2023
@longwuyuan
Copy link
Contributor

@iSerganov we now have Nginx v1.25 which has chunking on by default.
Someone else enabling chunking in their application code and the ingress-controller logged duplicate chunking config messages #11162

I am not sure if you are still interested in this but its been open without activity for a long time. So I will close this for now and you can reopen it if you are going to work on a PR as per notes in this issue history. thanks you

/close

@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Closing this issue.

In response to this:

@iSerganov we now have Nginx v1.25 which has chunking on by default.
Someone else enabling chunking in their application code and the ingress-controller logged duplicate chunking config messages #11162

I am not sure if you are still interested in this but its been open without activity for a long time. So I will close this for now and you can reopen it if you are going to work on a PR as per notes in this issue history. thanks you

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

3 participants