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

WAMimeType 'application/x-www-form-urlencoded' binary? #1453

Open
adriaon opened this issue Feb 3, 2025 · 4 comments
Open

WAMimeType 'application/x-www-form-urlencoded' binary? #1453

adriaon opened this issue Feb 3, 2025 · 4 comments

Comments

@adriaon
Copy link

adriaon commented Feb 3, 2025

Seaside considers the mime type 'application/x-www-form-urlencoded' to be binary. Shouldn't that be not binary?

@jbrichau
Copy link
Member

jbrichau commented Feb 4, 2025

I don't think it should be considered binary indeed. It's not very common to encounter it outside of http form submission requests, which are handled in a separate way, so that is probably why it has not really been an issue that WAMimeType>>isBinary does not correctly return false for it.

Could you maybe explain a bit more what context you encountered this as a problem? That might help in understanding a bit better to solve this.

@adriaon
Copy link
Author

adriaon commented Feb 4, 2025

Sure. I encountered this issue when working on general logging of HTTP requests. By 'general' I mean without any specific application context. The MIME type is used to determine which class (either a string class or byte array) to use for the payload, and if any decoding is required based on the charset property of the MIME type.

@jbrichau
Copy link
Member

jbrichau commented Feb 5, 2025

I guess the fix for WAMimeType>>#isBinary will be:

	"answers whether the contents of a document of the receiving mime type are binary"
	self main = 'text' ifTrue: [ ^ false ].
	self main = 'application'
		ifTrue: [
			"application/json and application/x-www-form-urlencoded are text"
			(#('json' 'x-www-form-urlencoded') includes: self sub) ifTrue: [ ^ false ] ].
	GRPlatform subStringsIn: self sub splitBy: $+ do: [ :each |
		"application/(x-)javascript and application/xml are text"
		(#('x-javascript' 'javascript' 'xml') includes: each)
			ifTrue: [ ^ false ] ].
	^ true

Does that work for you?

@adriaon
Copy link
Author

adriaon commented Feb 5, 2025

Looks good. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants