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

Imagor cannot handle commas in the filename and reverse proxy woes. #5

Open
Hotcooler opened this issue Oct 26, 2022 · 2 comments
Open

Comments

@Hotcooler
Copy link

Hotcooler commented Oct 26, 2022

So, to start there is a bug somewhere, commas as in ',' in the photo filename cause a crash. Probably on the imagor end, but things crash any way.

Basically this does not compute: http://imagor:8000/unsafe/meta/photos/00060-1597646341-invicible,_il.png it does fine without imagor though.

Also I could not make IMAGOR_SECRET: work at all, no matter if I had quotes or no quotes, spaces or something else, it would not even fire up local metadata rebuild. (That's the initial file detection and meta collection for the cache part).

DEBUG imagor/imagor.go:209 sign-mismatch {"params": {"path":"meta/photos/00041-3113491024-giant_scifi_s.png","image":"photos/00041-3113491024-giant_scifi_s.png","hash":"nic5S3sU80NEMe7Xr1ycuDQ1KyA=","meta":true}, "expected": "wf68iIiITTt0UjbQ9BJ_MfOTftY="}

Hashes would never match.

And I might be too wooden, but I cant for the life of me make the combo work via reverse proxy with https. Tried it with nginx and caddy (must admit a lot nicer..). Though it's a very weird kind of broken. (this is with IMAGOR_UNSAFE: 1, since I could not boot up otherwise, without pre-built cache that is).

I have a very simple reverse proxy (seems to work.. I did try messing with rewrites and headers before in nginx to try to make non unsafe one work, but to no avail, though granted I'm not that bright with that).

example.com  {
  reverse_proxy localhost:2070
  handle_path /img/* {
    reverse_proxy  localhost:2080
  }
}

with IMAGOR_CLIENT_BASE_URL: "https://example.com/img/"

And it kind-of works.. but, if there are any brackets ( or ) it suddenly does not. So if you ask for https://example.com/img/unsafe/828x552/filters:format(webp)/photos/00020-1594330263-postapocalypt.png you get error 400:

DEBUG imagor/imagor.go:295 load {"params": {"path":"828x552/filters:format%28webp%29/photos/00020-1594330263-postapocalypt.png","image":"filters:format(webp)/photos/00020-1594330263-postapocalypt.png","unsafe":true,"width":828,"height":552}, "error": "imagor: 400 invalid"}

Yet if you do not use any format(), everything works just dandy aka https://example.com/img/unsafe/828x552/photos/00020-1594330263-postapocalypt.png

DEBUG   imagor/imagor.go:522    suppress        {"key": "828x552/photos/00020-1594330263-postapocalypt.png"}
DEBUG   vips/processor.go:100   vips    {"log": "created imageRef 0xc000128dc0"}
INFO    vips/processor.go:102   VIPS    {"log": "selected loader is image source"}
INFO    vips/processor.go:102   VIPS    {"log": "input size is 768 x 512"}
INFO    vips/processor.go:102   VIPS    {"log": "loading with factor 1 pre-shrink"}
INFO    vips/processor.go:102   VIPS    {"log": "pre-shrunk size is 768 x 512"}
INFO    vips/processor.go:102   VIPS    {"log": "converting to processing space srgb"}
INFO    vips/processor.go:102   VIPS    {"log": "residual scale 1.07812 x 1.07812"}
INFO    vips/processor.go:102   VIPS    {"log": "converting to output space srgb"}
INFO    vips/processor.go:102   VIPS    {"log": "cropping to 828x552"}
DEBUG   vips/process.go:229     image   {"width": 828, "height": 552, "page_height": 552}
DEBUG   vips/processor.go:100   vips    {"log": "closing image 0xc000128dc0"}
DEBUG   vips/processor.go:100   vips    {"log": "closing source 0xc000128d40"}
DEBUG   imagor/imagor.go:329    processed       {"params": {"path":"828x552/photos/00020-1594330263-postapocalypt.png","image":"photos/00020-1594330263-postapocalypt.png","unsafe":true,"width":828,"height":552}}
DEBUG   imagor/imagor.go:482    saved   {"key": "828x552/photos/00020-1594330263-postapocalypt.png"}

If you ask for non-existent file and request has brackets - you still get 400 instead of 404, so must be the brackets..

imagor/imagor.go:295 load {"params": {"path":"828x552/filters:format%28webp%29/photos/00020-1594330263-postapocalypt.webp","image":"filters:format(webp)/photos/00020-1594330263-postapocalypt.webp","unsafe":true,"width":828,"height":552}, "error": "imagor: 400 invalid"}

Result seems to be the same in both nginx and caddy.
Yet brackets work just fine via the http port (though have not tried to get it through reverse-proxy in a non-https manner).

The proxy stuff is likely a user-error of some kind since I never really messed much with stuff that has similar backends (though tips would be appreciated), but IMAGOR_SECRET seems to be broken in some way or the other. ( If it's not me beeing dumb, then it's either wrong path gets hashed, which I imagine will be true with reverse proxy to /img/, though it seems to me that only part of the URL get hashed, and it would not really matter what host or port it was on, but we did not get that far haha, so probably something to do with "excluding /unsafe/".

@Inlustra
Copy link
Owner

Inlustra commented Oct 26, 2022

There's a lot to unpack here.

The issue with the comma not working if you go directly to Imagor, should probably be raised with Imagor, I did some quick testing and can confirm it at least.

With the Imagor hash... I had not actually tested that, so that's on me, however the conclusion I've come to is probably not something you're going to like!

What I realised is that the client needs to be able to generate the Imagor URLs itself as the sizes requested are completely dependent on client. /500x500 etc.
My thoughts there were that maybe I could create a redirect endpoints so the URL for the image would become something like:
/api/thumbnail?photo='...'&width=500&height=500 -----> IMAGOR_BASE/HASH/500x500/...

However that's almost as insecure as not having a secret at all. Essentially I don't think we're going to be able to support IMAGOR_SECRET but I'd love your input.

@Hotcooler
Copy link
Author

Hotcooler commented Oct 26, 2022

Realistically for my use case, no backend works fine to be honest, it's just more for testing purposes. I thought about the fact that image sizes in cache json are static and requested sizes must depend on display size, but figured there might be something solving for that (have not really looked in any depth at the code since I'm well.. not well versed in that). I guess not. But it does not really explain why the meta requests were also failing when initially building the cache.

And I also cannot really think about any easy solution. Though I'd say some end point with security-through-obscurity would be better than nothing. That would allow to completely hide imagor from the web and only have communication inside docker network, as in not expose the api directly, and probably filter for limited options like no resizing bigger than source image e.t.c. Plus it would be probably a good idea to hash/obfuscate the request to some extent even on the client side. Say seeing api/thumbnail?photo='sdlht24hg938w@TSDT4sdg' in the URL will likely discourage/solve a lot of potential issues with automated DDOS e.t.c. May be some sort of rate-limiting too.

I'll ping the imagor people with the comma stuff later, will grab some logs e.t.c.

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