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

Address non-ASCII filename support for files, audio, etc. #75

Merged
merged 3 commits into from
Jun 19, 2024

Conversation

trrwilson
Copy link
Collaborator

Fixes #72.

The library's multipart/form-data implementation uses custom logic to bypass the use of the filename* Content-Disposition parameter, which is RFC 6266's method of encoding non-ASCII filenames in multipart/form-data requests. This bypass was made in deference to an interpretation of RFC 7578 wherein the use of filename* is disallowed.

This PR removes the restriction by directly using System.Net.Http.MultipartFormDataContent.Add()'s filename* behavior.

Although pending discussion with the Base Common Library team is still needed to clarify best interoperability practices, this is a better current state because:

  • It aligns with the existing language type in System.Net.Http
  • The interpretation of the competing RFC is ambiguous; it appears it may not be making a statement about the filename* parameter itself, but rather on the use of the encoding scheme of the newer filename* in the older filename
  • Most importantly, it fixes the readily observable issue against the OpenAI V1 endpoint -- and still leaves the older filename intact for any unanticipated backwards compatibility issues with API gateways or other intermediaries

src/Utility/MultipartFormDataBinaryContent.cs Outdated Show resolved Hide resolved
@trrwilson trrwilson merged commit 2ba8e69 into main Jun 19, 2024
1 check passed
@trrwilson trrwilson deleted the user/travisw/mpfd-unicode-filenames branch June 19, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants