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

[asgi] Headers with special characters cause the request to fail #1814

Closed
gaganpreet opened this issue May 19, 2023 · 4 comments · Fixed by #2837
Closed

[asgi] Headers with special characters cause the request to fail #1814

gaganpreet opened this issue May 19, 2023 · 4 comments · Fixed by #2837
Labels
bug Something isn't working

Comments

@gaganpreet
Copy link

Hi, I noticed some alerts pouring in on Sentry while setting up OpenTelemetry integration.

TL;DR: A FastAPI instrumented app will throw an error with the following request:
curl 'http://localhost:1234' -H $'X-Custom-Header: \xe9'

I dug deeper and found that this issue does not exist in the Flask instrumentation. Werkzeug's implementation uses http.client.parse_headers which has already decoded headers as iso-8859-1 before being passed to the instrumentor. Meanwhile, the ASGI instrumentor receives headers as bytes and incorrectly tires to decode headers as utf-8 which causes this error.

Describe your environment

Python version: 3.10.10
OS: Manjaro

Steps to reproduce

server.py

server.py
import uvicorn
from fastapi import FastAPI
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor
from opentelemetry.trace import (
    set_tracer_provider,
)
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import (
    SimpleSpanProcessor,
    ConsoleSpanExporter,
)

provider = TracerProvider()
provider.add_span_processor(SimpleSpanProcessor(ConsoleSpanExporter()))
set_tracer_provider(provider)


app = FastAPI()

FastAPIInstrumentor.instrument_app(app)


@app.get("/")
def read_root():
    return {"Hello": "World"}


if __name__ == "__main__":
    uvicorn.run(app, host="localhost", port=1234)

client.py

client.py
import requests
requests.get('http://localhost:1234', headers={'custom-header': 'é'}).text

Curl equivalent: curl 'http://localhost:1234' -H $'X-Custom-Header: \xe9'

What is the expected behavior?

Client side input shouldn't cause the request to fail.

What is the actual behavior?

Request fails with "Internal Server Error"

Traceback
Traceback (most recent call last):
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/opentelemetry/instrumentation/asgi/__init__.py", line 539, in __call__
    custom_attributes = collect_custom_request_headers_attributes(
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/opentelemetry/instrumentation/asgi/__init__.py", line 342, in collect_custom_request_headers_attributes
    headers = {
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/opentelemetry/instrumentation/asgi/__init__.py", line 343, in <dictcomp>
    _key.decode("utf8"): _value.decode("utf8")
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe9 in position 0: unexpected end of data

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 428, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/fastapi/applications.py", line 276, in __call__
    await super().__call__(scope, receive, send)
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/starlette/applications.py", line 122, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/starlette/middleware/errors.py", line 184, in __call__
    raise exc
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/starlette/middleware/errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "/home/gagan/.virtualenvs/otel/lib/python3.10/site-packages/opentelemetry/instrumentation/asgi/__init__.py", line 565, in __call__
    duration = max(round((default_timer() - start) * 1000), 0)
UnboundLocalError: local variable 'start' referenced before assignment
@gaganpreet gaganpreet added the bug Something isn't working label May 19, 2023
@gaganpreet
Copy link
Author

I'm happy to submit a PR to fix this bug. However, I'm not sure what's the right fix is here. Changing decode from utf-8 to iso-8859-1 would be ideal but it might break backwards compatibility.

@srikanthccv
Copy link
Member

This is probably a duplicate of #1478, and there is a PR for it #1713

thomasleveil added a commit to thomasleveil/opentelemetry-python-contrib that referenced this issue Jul 12, 2023
@drobert
Copy link

drobert commented Aug 16, 2024

This issue is biting my product as well; seems like the PR has been up in 'draft' for well over a year now. How can we get this through the finish line?

@lzchen
Copy link
Contributor

lzchen commented Aug 19, 2024

@drobert

Seems like the original author of the PR is no longer working on it. Anyone can feel free to open up a pr to address this issue.

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

Successfully merging a pull request may close this issue.

4 participants