Use text/plain Content-Type if the Content-type is present but invalid #53

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@vdaron
Contributor

vdaron commented Jan 24, 2012

Pull Request to fix bug : DR-93 Invalid server Content-Type should'nt return HTTP Status: UnableToConnect

Thanks

@bjorg

This comment has been minimized.

Show comment Hide comment
@bjorg

bjorg Jan 24, 2012

Contributor

What is the motivation for this fix? If I recall correctly, in other places when we don't know the MimeType, we use 'application/octect-stream' instead (which is a more opaque mime-type).

Contributor

bjorg commented Jan 24, 2012

What is the motivation for this fix? If I recall correctly, in other places when we don't know the MimeType, we use 'application/octect-stream' instead (which is a more opaque mime-type).

@vdaron

This comment has been minimized.

Show comment Hide comment
@vdaron

vdaron Jan 24, 2012

Contributor

I'm connecting to a server that doesn't respect standards and returned me "Content-Type:xml" in it's REST api.

Without that fix, I can't call any webservice method, I have an UnableToConnect error.

application/octect-stream instead of text/plain is fine for me.

Contributor

vdaron commented Jan 24, 2012

I'm connecting to a server that doesn't respect standards and returned me "Content-Type:xml" in it's REST api.

Without that fix, I can't call any webservice method, I have an UnableToConnect error.

application/octect-stream instead of text/plain is fine for me.

@bjorg

This comment has been minimized.

Show comment Hide comment
@bjorg

bjorg Jan 27, 2012

Contributor

Use application/octect-stream then. Otherwise, the risk is that you'll get the text with the wrong Encoding. This way, it's clear that you have no idea what text encoding should be used since it's a byte stream.

Contributor

bjorg commented Jan 27, 2012

Use application/octect-stream then. Otherwise, the risk is that you'll get the text with the wrong Encoding. This way, it's clear that you have no idea what text encoding should be used since it's a byte stream.

@vdaron

This comment has been minimized.

Show comment Hide comment
@vdaron

vdaron Jan 27, 2012

Contributor

MimeType.DefaultMimeType looks even more consistent and equals MimeType.BINARY

Contributor

vdaron commented Jan 27, 2012

MimeType.DefaultMimeType looks even more consistent and equals MimeType.BINARY

@bjorg

View changes

src/mindtouch.dream/plug/HttpPlugEndpoint.cs
+
+ // Let's default to TEXT if the ContentType is invalid
+ if(!string.IsNullOrEmpty(httpResponse.ContentType) && !MimeType.TryParse(httpResponse.ContentType, out contentType)) {
+ _log.WarnFormat("Invalid Content Type : '{0}', using plain/text as default",httpResponse.ContentType);

This comment has been minimized.

Show comment Hide comment
@bjorg

bjorg Jan 28, 2012

Contributor

log comment is wrong it should now say application/octect-stream (maybe print out the value of DefaultMimeType instead of hard-coding); also, missing space after comma

@bjorg

bjorg Jan 28, 2012

Contributor

log comment is wrong it should now say application/octect-stream (maybe print out the value of DefaultMimeType instead of hard-coding); also, missing space after comma

@bjorg

View changes

src/mindtouch.dream/plug/HttpPlugEndpoint.cs
+ if(!string.IsNullOrEmpty(httpResponse.ContentType) && !MimeType.TryParse(httpResponse.ContentType, out contentType)) {
+ _log.WarnFormat("Invalid Content Type : '{0}', using plain/text as default",httpResponse.ContentType);
+ contentType = MimeType.DefaultMimeType;
+ }

This comment has been minimized.

Show comment Hide comment
@bjorg

bjorg Jan 28, 2012

Contributor

great idea. much better!

@bjorg

bjorg Jan 28, 2012

Contributor

great idea. much better!

@bjorg bjorg closed this Nov 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment