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

Add proxy support to native fetcher #590

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@tcpper
Copy link

tcpper commented Jan 2, 2014

Example:
pagespeed FetchProxy 127.0.0.1:8081;
pagespeed FetchProxy www.proxy.com:8081:

By replace the ip address which nginx connect to with the ip address of the proxy server and replace the dns name nginx take to resolve with the dns name of the proxy server, we manage to pass all the fetch traffic to the proxy server. BTW, i am not very familiar with http proxy and github, so careful review is needed ^_^

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 2, 2014

@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 2, 2014

@dinic could you have a look at this change?

@@ -106,6 +106,36 @@
}
}

// TODO(tcpper): maybe we should reform NgxFetch and NgxUrlAsyncFetcher to use the same ParseUrl

This comment has been minimized.

@oschaaf

oschaaf Jan 3, 2014

Member

I agree - ParseUrl() should be refactored for re-use.

@@ -192,7 +222,7 @@
MessageHandler* message_handler,
AsyncFetch* async_fetch) {
async_fetch = EnableInflation(async_fetch, NULL);
NgxFetch* fetch = new NgxFetch(url, async_fetch,
NgxFetch* fetch = new NgxFetch(url_/*proxy*/, url, async_fetch,

This comment has been minimized.

@oschaaf

oschaaf Jan 3, 2014

Member

It would be more readable if url_ was named proxy_

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jan 3, 2014

FWIW: code duplication and naming of url_ aside, this LGTM

tcpper
modify NgxUrlAsyncFetcher and NgxFetch to use the same static ParseUr…
…l and rename NgxUrlAsyncFetcher's url_ to proxy_
@tcpper

This comment has been minimized.

Copy link

tcpper commented Jan 3, 2014

hi @oschaaf , i've made some change according to your comments ~

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jan 3, 2014

@tcpper great, LGTM!

jeffkaufman added a commit that referenced this pull request Jan 8, 2014

native-fetcher: add support for FetchProxy
The native fetcher previously ignored FetchProxy settings; now it doesn't.

Squash-merge of tcpper's #590.
@jeffkaufman

This comment has been minimized.

Copy link
Contributor

jeffkaufman commented Jan 8, 2014

squash-merged as df57366

Thanks @tcpper!

This should be in release 1.8 when we make it, and people can use it with 1.7 by building from master.

@tcpper

This comment has been minimized.

Copy link

tcpper commented Jan 9, 2014

@jeffkaufman great!

@tcpper tcpper closed this Jan 9, 2014

@jmaessen jmaessen changed the title add proxy support to native fetcher Add proxy support to native fetcher Apr 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment