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

Add proxy server feature. #41

Closed

Conversation

Darkclainer
Copy link
Contributor

Now in configuration in "proxy" section you can specify two new options:

  1. url - is a proxy url
  2. mode - it is string, one of "all", "missing", "none"

Issue #33

Now in configuration in "proxy" section you can specify two new options:
1. url - is a proxy url
2. mode - it is string, one of "all", "missing", "none"
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

type ProxyMode int

const (
// Proxy server is off
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existing comment doesn't match identifier "ProxyNone"

Suggested change
// Proxy server is off
// ProxyNone server is off

const (
// Proxy server is off
ProxyNone ProxyMode = iota
// Only missing requests are proxied
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existing comment doesn't match identifier "ProxyMissing"

Suggested change
// Only missing requests are proxied
// ProxyMissing missing requests are proxied

ProxyNone ProxyMode = iota
// Only missing requests are proxied
ProxyMissing
// All requests are proxied
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existing comment doesn't match identifier "ProxyAll"

Suggested change
// All requests are proxied
// ProxyAll requests are proxied

@codecov-io
Copy link

codecov-io commented Oct 21, 2019

Codecov Report

Merging #41 into master will increase coverage by 1.3%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #41     +/-   ##
========================================
+ Coverage      91%   92.3%   +1.3%     
========================================
  Files           6       7      +1     
  Lines         189     221     +32     
========================================
+ Hits          172     204     +32     
  Misses         14      14             
  Partials        3       3
Impacted Files Coverage Δ
internal/server/http/server.go 86.58% <100%> (+1.65%) ⬆️
internal/config.go 100% <100%> (ø) ⬆️
internal/server/http/proxy.go 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cca6b9c...171cc14. Read the comment docs.

@aperezg aperezg requested review from aperezg and joanlopez October 26, 2019 20:05
@aperezg
Copy link
Member

aperezg commented Oct 26, 2019

Can you check it, @joanlopez?

}

// ProxyMode is enumeration of proxy server modes
type ProxyMode int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type ProxyMode int
type ProxyMode uint8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need specifically uint8?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we prefer declare the enums with a more accurate type possible as standard, 8bits is more than enough to a enum and in most case u don't need negative numbers, so uint8 is a elegant solution

internal/config.go Show resolved Hide resolved
@friendsofgo friendsofgo deleted a comment Nov 2, 2019
// Handler returns handler that sends request to another server.
func (p *Proxy) Handler() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
p.server.ServeHTTP(w, r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to update the headers to allow for SSL redirection:

                r.URL.Host = p.url.Host
		r.URL.Scheme = p.url.Scheme
		r.Header.Set("X-Forwarded-Host", r.Header.Get("Host"))
		r.Host = p.url.Host

For that you will need to persist the result of url.Parse into the proxy struct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with the comment that I suggested and, all working very fine :)

@aperezg
Copy link
Member

aperezg commented Nov 9, 2019

All options that we include on the config file, usually are on the command line also.

I think that you could add something like that:
-proxy-url="https://swapi.co" -proxy-mode=missing

And if the proxy-mode is not declare using by default none, and if the proxy_mode != none and proxy_url is not defined get an early error and close the application.

@aperezg
Copy link
Member

aperezg commented Nov 9, 2019

Could you update the README.md with how to use the proxy?

In the section Using Killgrave

Below to the sentence: "Or custome your server with this flags:" we're printing an output of the help command, so when you've added your new flags, you will need to update this with your output, as now.

And in the section "Use killgrave with config file:"

Modify the example with the proxy, i.e:

#config.yml

imposters_path: "imposters"
port: 3000
host: "localhost"
proxy:
  url: https://example.com
  mode: missing
cors:
  methods: ["GET"]
  headers: ["Content-Type"]
  exposed_headers: ["Cache-Control"]
  origins: ["*"]
  allow_credentials: true

And if you want you could include a section like #CORS section, where you explain how the proxy works and which are the options to configure the proxy.

Thanks!

@aperezg aperezg mentioned this pull request Jan 3, 2020
@aperezg
Copy link
Member

aperezg commented Jan 3, 2020

Follow #44

@aperezg aperezg closed this Jan 3, 2020
@aperezg aperezg mentioned this pull request Jan 30, 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.

3 participants