Alamofire / AlamofireImage

AlamofireImage is an image component library for Alamofire
MIT License
3.98k stars 523 forks source link

If the image doesn't have a extension name, it will not be set #28

Closed kevinzhow closed 9 years ago

kevinzhow commented 9 years ago

For example

https://park-topic-attachments.s3.cn-north-1.amazonaws.com.cn/dea34f3d-7a19-42c0-9d97-d4579544688c

this image has mime type but extension name, af_setImageWithURL will not work on it

cnoon commented 9 years ago

The reason this doesn't work is that the content-type coming back is not a valid image type:

cnoon:~$ curl -I https://park-topic-attachments.s3.cn-north-1.amazonaws.com.cn/dea34f3d-7a19-42c0-9d97-d4579544688c
HTTP/1.1 200 OK
x-amz-id-2: 2oZAbYN6c6nSuwkMlFsBWF4kWTUsV3MhIx1pVmXOy5opUECq9m5TPWTJdXCRc5t0
x-amz-request-id: A3DA392439695D25
Date: Wed, 07 Oct 2015 03:03:07 GMT
Last-Modified: Fri, 02 Oct 2015 17:58:06 GMT
ETag: "f0be121795b802cfa5ed6479c5d59fcd"
Accept-Ranges: bytes
Content-Type: binary/octet-stream
Content-Length: 178051
Server: AmazonS3

This causes the content type validation step to fail internally. If you don't run the validation check on the response content type, the image is deserialized properly. I think you've uncovered an edge case that we don't handle as well as we could.

What I'm thinking at the moment is to add an additional flag to the responseImage serializer:

public func responseImage(
    imageScale: CGFloat = Request.imageScale,
    validateContentType: Bool = true,
    inflateResponseImage: Bool = true,
    completionHandler: Response<Image, NSError> -> Void)
    -> Self
{
    // TODO: pass to image serializer
}

If you disable this, then you'll hit a decoding error if the image is not actually an image that UIKit can decode.

Thoughts @kevinzhow???

cnoon commented 9 years ago

cc @kcharwood for a second opinion.

cnoon commented 9 years ago

@kcharwood pointed me to AFNetworking #199 and AFNetworking #228 to help provide some additional context. I'm going to think on this approach a bit more before deciding exactly how to solve it properly.

At the moment, I'm of the mindset that we probably shouldn't be disabling validation, but instead allowing users to specify additional acceptable content-types.

cnoon commented 9 years ago

Pull request #33 adds the ability to specify additional acceptable image content types for a Request. I'm going to close this issue out. Please redirect all further comments to #33.