-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
proposal: image: support decoder instance with specific formats #20814
Comments
"Right now the only way to prevent your profile picture upload handler from using the tiff package is to reimplement a large part of https://golang.org/src/image/format.go." Well, format.go is only 100 lines of code, including comments, so a large part of that is still a small amount of code. If I was writing a program that should decode only JPEG and PNG but not TIFF, then I'd just do that. Or even just hard code a 10 line function that's: switch on the first byte, case 0x89: try png.Decode, case 0xff: try jpeg.Decode. |
zip does this with RegisterDecompressor (global) vs Reader.RegisterDecompressor (local to Reader), but there's no equivalent of Reader yet in the image API. We'd have to add one. |
@rsc the zip package looks like a good example except for this behaviour. Defaulting to looking up package defaults ruins the increased security you'd get from manually registering image decoders. It might also be something to look at in the https://golang.org/pkg/archive/zip/#Reader.RegisterDecompressor
|
Honestly I'm kind of skeptical of the "increased security". If you really want to decode only a few formats, you can sniff the input types. Registering alternate decoders seems more useful. Also, if you really do want the security you could register a decoder with magic="". |
I don't think there's a forward path I find valuable. The current situation is fine with me and the problem seems minor and easy to work around with the existing API. |
I agree with @robpike. |
Based on the discussion above, this seems to have limited value and is possible to do as a helper package outside the standard library. I'd encourage anyone interested to start by doing that. |
Today canonical way to register formats for the image package is to import a package which calls
image.RegisterFormat
. This is simple and works fine for small programs however when your programs start to grow there is no way to ensure that theimage.Decode
function will only attempt to decode a predefined list of formats.Example scenario
You have a profile picture system which you want to accept jpeg, and png but you also have an admin page which requires reading tiff images. The tiff image package is still experimental and you would prefer not to expose it to external users who just need to upload a profile picture in common formats.
Right now the only way to prevent your profile picture upload handler from using the tiff package is to reimplement a large part of
https://golang.org/src/image/format.go
. The only other option is to write separate programs which may or may not make sense or be easy depending on your ops.In short this is a security(limiting exposure to certain image decoders) and developer usability issue.
Required additions/API
The required additions aren't very large but the api is important since this will be here for a while(go1 compat). The real issue is not implementing this, it is agreeing upon how this should work and look. The following is a rough outline for what would need to be added.
Decoder
type similar to json.Decoder which can accept a list of formats to look for and accept.image.RegisterFormat
publicly available in image decoder packagesimage.format
public so that each image decoder package could definePossible api and usage
Changes required for image format package(image/png, image/jpeg, etc). Replaces
init()
with something like this https://golang.org/src/image/png/reader.go#L1017Usage for a user wanting to only use specific formats without relying on all other packages to not import other image format packages which automatically register themselves on the package level decoder.
The text was updated successfully, but these errors were encountered: