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

WIP Adding proxying support #40

Merged
merged 23 commits into from
Oct 30, 2019
Merged

WIP Adding proxying support #40

merged 23 commits into from
Oct 30, 2019

Conversation

karl-ravn
Copy link
Contributor

Sometimes this service needs to be hidden behind friendly lines and not exposing the exact URL for the favicon resource that was requested. Sometimes you get mixed results when your own service runs https but the favicon is supposed to be downloaded from http - causing everything to break and emit errors.

Running the server with DOWNLOAD_MODE=proxy makes the server download and forward the resource to the user.

@galgolan
Copy link

galgolan commented Jun 4, 2019

This is a great and useful feature!
I started the code with DOWNLOAD_MODE=proxy but the server behaves the same...

@karl-ravn
Copy link
Contributor Author

Sorry for not responding back earlier. When the server is started with DOWNLOAD_MODE=proxy, it's supposed to almost behave the same. However, there are two main differences:

with current non-proxy mode:

59001:~ karrav$ curl --head http:'//localhost:3000/icon?url=www.marketwatch.com&size=32'
HTTP/1.1 302 Found
Cache-Control: max-age=2592000
Content-Type: text/html; charset=utf-8
Location: https://mw4.wsj.net/mw5/content/logos/favicon.ico
Date: Thu, 05 Sep 2019 11:43:17 GMT

with proxy-mode:

59001:~ karrav$ curl --head http:'//localhost:3000/icon?url=www.marketwatch.com&size=32'
HTTP/1.1 200 OK
Cache-Control: max-age=2592000
Date: Thu, 05 Sep 2019 11:39:17 GMT
Content-Type: image/png

The main difference here would be that the icon binary will be sent over the request instead of being redirected to there.

@mat
Copy link
Owner

mat commented Sep 15, 2019

Thanks @karl-ravn for this contribution and sorry for getting back so late :(

I, too, think this is useful for some people.

Some suggestion before we merge:

@karl-ravn karl-ravn changed the title Adding proxying support WIP Adding proxying support Oct 21, 2019
@karl-ravn
Copy link
Contributor Author

Ok, I've done 1,2 and 4 but writing a good unit test for this... it's a bit tricky since I'm not really a Go developer..

@immanuelfodor
Copy link

I've just found this project searching for a favicon proxy, this PR would be so great to have :)

@mat mat merged commit 258777f into mat:master Oct 30, 2019
@mat
Copy link
Owner

mat commented Oct 30, 2019

Thanks @karl-ravn for adding this feature.

I'm going to add a test and will release a new version soon, too.

mat added a commit that referenced this pull request Oct 30, 2019
@csandman
Copy link

@mat is there any way to access this var when using the docker image? It appears its not currently available, and I don't see SERVER_MODE in the docker file.

@mat
Copy link
Owner

mat commented Dec 10, 2019

@csandman Good catch, I failed to release a new version of the Docker image! The latest docker image is now up to date and contains support for SERVER_MODE: https://hub.docker.com/r/matthiasluedtke/iconserver/latest

mat added a commit that referenced this pull request Jul 23, 2020
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.

5 participants