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

Set default content-type to 'text/html' #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Set default content-type to 'text/html' #8

wants to merge 1 commit into from

Conversation

DoWhileGeek
Copy link

The Mime library defaults to 'application/octet-stream', but for folks
who want extentionless urls served by s3, you have to take the
extention off the filename.

Resolves #7.

The Mime library defaults to 'application/octet-stream', but for folks
who want extentionless urls served by s3, you have to take the
extention off the filename.
@jamestalmage
Copy link
Owner

I would rather set this as a config parameter instead of making this assumption automatically.

I would accept a PR that did the following things:

  • export mime.define so users on this module so users can add custom mime types.
  • reliably detect when mime can't find a type and return config.defaultMimeType
  • adds a mimeType parameter to aws-upload.conf.js. This must be a function that takes a absolute file path as it's only argument and returns a mime type string or null. If null is returned, the default mime type lookup will be used.
  • adds unit tests for both the above.
  • adds documentation for both the above.

@DoWhileGeek
Copy link
Author

@jamestalmage I did some testing with the mime library to test the mapping you suggested, and empty strings are not mapped the way you are wanting.

> mime.define({"text/html": ["", "html"]})
undefined
> mime.lookup("index")
'application/octet-stream'
> mime.lookup("")
'text/html'

Not to mention, this mapping is a little out of scope on what I wanted to help with. How do you feel instead about defaultMimeType that takes in a string that mime defaults to if it cant find anything?

@jamestalmage
Copy link
Owner

I was simply suggesting we write our own implementation in from of the mime library. I didn't even realize mime.define existed. That changes things a bit. See the updated description.

How would we decide that mime "couldn't find anything"? It returns application/octet-stream by default, but what if someone did this:

mime.define('application/octest-stream': ['my-custom-ext'])

Then anything with that custom extension will also return application/octest-stream, but that doesn't mean we don't know what it did.

I'd probably wrap in a try / finally:

var originalDefaultType = mime.default_type
try {
   mime.default_type = null; // or some reasonably unique string if setting to null causes problems.
   var mimeType =  mime(ext);
   return mimeType || conf.defaultMimeType;
} finally {
  mime.default_type = originalDefaultType;
}

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

Successfully merging this pull request may close these issues.

2 participants