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

Fix X-Forwarded-* headers in openvsx-proxy #12071

Merged
merged 1 commit into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions components/openvsx-proxy/pkg/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ func (o *OpenVSXProxy) Handler(p *httputil.ReverseProxy) func(http.ResponseWrite
key, err := o.key(r)
if err != nil {
log.WithFields(logFields).WithError(err).Error("cannot create cache key")
r.Host = o.upstreamURL.Host
Copy link
Member

Choose a reason for hiding this comment

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

it seems to break: #12071 (comment)

Do we really need to change it? OpenVSX should care about X-Forwarded* to create links or it also looks at Host header for it?

Copy link
Member

@akosyakov akosyakov Aug 12, 2022

Choose a reason for hiding this comment

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

It is interesting though in context of Self Hosted in air gapped scenario where maybe a user machine does not have access to open-vsx.org directly but only via our proxy. In this case it would be even desirable to navigate user to our proxy for access of website. Not sure. cc @corneliusludmann @mrsimonemms

p.ServeHTTP(rw, r)
o.finishLog(logFields, start, hitCacheRegular, hitCacheBackup)
o.metrics.DurationRequestProcessingHistogram.Observe(time.Since(start).Seconds())
Expand Down Expand Up @@ -105,7 +104,6 @@ func (o *OpenVSXProxy) Handler(p *httputil.ReverseProxy) func(http.ResponseWrite
log.WithFields(logFields).WithFields(o.DurationLogFields(duration)).Info("processing request finished")
o.metrics.DurationRequestProcessingHistogram.Observe(duration.Seconds())

r.Host = o.upstreamURL.Host
p.ServeHTTP(rw, r)
o.finishLog(logFields, start, hitCacheRegular, hitCacheBackup)
}
Expand Down
51 changes: 4 additions & 47 deletions components/openvsx-proxy/pkg/modifyresponse.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ package pkg

import (
"bytes"
"compress/gzip"
"fmt"
"io/ioutil"
"net/http"
"strconv"
"strings"
"time"
"unicode/utf8"

Expand Down Expand Up @@ -55,7 +53,6 @@ func (o *OpenVSXProxy) ModifyResponse(r *http.Response) error {
return err
}
r.Body.Close()
r.Body = ioutil.NopCloser(bytes.NewBuffer(rawBody))

if r.StatusCode >= 500 || r.StatusCode == http.StatusTooManyRequests || r.StatusCode == http.StatusRequestTimeout {
// use cache if exists
Expand Down Expand Up @@ -94,49 +91,9 @@ func (o *OpenVSXProxy) ModifyResponse(r *http.Response) error {
}

// no error (status code < 500)
body := rawBody
contentType := r.Header.Get("Content-Type")
if strings.HasPrefix(contentType, "application/json") {
isCompressedResponse := strings.EqualFold(r.Header.Get("Content-Encoding"), "gzip")
if isCompressedResponse {
gzipReader, err := gzip.NewReader(ioutil.NopCloser(bytes.NewBuffer(rawBody)))
if err != nil {
log.WithFields(logFields).WithError(err)
return nil
}

body, err = ioutil.ReadAll(gzipReader)
if err != nil {
log.WithFields(logFields).WithError(err).Error("error reading compressed response body")
return nil
}
gzipReader.Close()
}

if log.Log.Level >= logrus.DebugLevel {
log.WithFields(logFields).Debugf("replacing %d occurence(s) of '%s' in response body ...", strings.Count(string(body), o.Config.URLUpstream), o.Config.URLUpstream)
}
bodyStr := strings.ReplaceAll(string(body), o.Config.URLUpstream, o.Config.URLLocal)
body = []byte(bodyStr)

if isCompressedResponse {
var b bytes.Buffer
gzipWriter := gzip.NewWriter(&b)
_, err = gzipWriter.Write(body)
if err != nil {
log.WithFields(logFields).WithError(err).Error("error writing compressed response body")
return nil
}
gzipWriter.Close()
body = b.Bytes()
}
} else {
log.WithFields(logFields).Debugf("response is not JSON but '%s', skipping replacing '%s' in response body", contentType, o.Config.URLUpstream)
}

cacheObj := &CacheObject{
Header: r.Header,
Body: body,
Body: rawBody,
StatusCode: r.StatusCode,
}
err = o.StoreCache(key, cacheObj)
Expand All @@ -146,8 +103,8 @@ func (o *OpenVSXProxy) ModifyResponse(r *http.Response) error {
log.WithFields(logFields).Info("successfully stored response to cache")
}

r.Body = ioutil.NopCloser(bytes.NewBuffer(body))
r.ContentLength = int64(len(body))
r.Header.Set("Content-Length", strconv.Itoa(len(body)))
r.Body = ioutil.NopCloser(bytes.NewBuffer(rawBody))
r.ContentLength = int64(len(rawBody))
r.Header.Set("Content-Length", strconv.Itoa(len(rawBody)))
return nil
}
85 changes: 0 additions & 85 deletions components/openvsx-proxy/pkg/openvsxproxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package pkg

import (
"bytes"
"compress/gzip"
"fmt"
"io"
"net/http"
Expand All @@ -30,90 +29,6 @@ func createFrontend(backendURL string) (*httptest.Server, *OpenVSXProxy) {
return frontend, openVSXProxy
}

func TestReplaceHostInJSONResponse(t *testing.T) {
backend := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
bodyBytes, _ := io.ReadAll(r.Body)
rw.Header().Set("Content-Type", "application/json")
rw.Write([]byte(fmt.Sprintf("Hello %s!", string(bodyBytes))))
}))
defer backend.Close()

frontend, _ := createFrontend(backend.URL)
defer frontend.Close()

frontendClient := frontend.Client()

requestBody := backend.URL
req, _ := http.NewRequest("POST", frontend.URL, bytes.NewBuffer([]byte(requestBody)))
req.Close = true
res, err := frontendClient.Do(req)
if err != nil {
t.Fatal(err)
}
expectedResponse := fmt.Sprintf("Hello %s!", frontend.URL)
if bodyBytes, _ := io.ReadAll(res.Body); string(bodyBytes) != expectedResponse {
t.Errorf("got body '%s'; expected '%s'", string(bodyBytes), expectedResponse)
}
}

func TestReplaceHostInCompressedJSONResponse(t *testing.T) {
backend := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
bodyBytes, _ := io.ReadAll(r.Body)
rw.Header().Set("Content-Type", "application/json")
rw.Header().Set("Content-Encoding", "gzip")

var b bytes.Buffer
w := gzip.NewWriter(&b)
w.Write([]byte(fmt.Sprintf("Hello %s!", string(bodyBytes))))
w.Close()
rw.Write(b.Bytes())
}))
defer backend.Close()

frontend, _ := createFrontend(backend.URL)
defer frontend.Close()

frontendClient := frontend.Client()

requestBody := backend.URL
req, _ := http.NewRequest("POST", frontend.URL, bytes.NewBuffer([]byte(requestBody)))
req.Close = true
res, err := frontendClient.Do(req)
if err != nil {
t.Fatal(err)
}
expectedResponse := fmt.Sprintf("Hello %s!", frontend.URL)
if bodyBytes, _ := io.ReadAll(res.Body); string(bodyBytes) != expectedResponse {
t.Errorf("got body '%s'; expected '%s'", string(bodyBytes), expectedResponse)
}
}

func TestNotReplaceHostInNonJSONResponse(t *testing.T) {
backend := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
bodyBytes, _ := io.ReadAll(r.Body)
rw.Header().Set("Content-Type", "application/octet-stream")
rw.Write([]byte(fmt.Sprintf("Hello %s!", string(bodyBytes))))
}))
defer backend.Close()

frontend, _ := createFrontend(backend.URL)
defer frontend.Close()

frontendClient := frontend.Client()

requestBody := backend.URL
req, _ := http.NewRequest("POST", frontend.URL, bytes.NewBuffer([]byte(requestBody)))
req.Close = true
res, err := frontendClient.Do(req)
if err != nil {
t.Fatal(err)
}
expectedResponse := fmt.Sprintf("Hello %s!", backend.URL)
if bodyBytes, _ := io.ReadAll(res.Body); string(bodyBytes) != expectedResponse {
t.Errorf("got body '%s'; expected '%s'", string(bodyBytes), expectedResponse)
}
}

func TestAddResponseToCache(t *testing.T) {
backend := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
bodyBytes, _ := io.ReadAll(r.Body)
Expand Down
79 changes: 78 additions & 1 deletion components/openvsx-proxy/pkg/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"net/http/httputil"
"net/url"
"strings"
"time"

"github.com/eko/gocache/cache"
Expand Down Expand Up @@ -58,7 +59,7 @@ func (o *OpenVSXProxy) Start() (shutdown func(context.Context) error, err error)
return nil, err
}
}
proxy := httputil.NewSingleHostReverseProxy(o.upstreamURL)
proxy := newSingleHostReverseProxy(o.upstreamURL)
proxy.ErrorHandler = o.ErrorHandler
proxy.ModifyResponse = o.ModifyResponse
proxy.Transport = &DurationTrackingTransport{o: o}
Expand Down Expand Up @@ -112,3 +113,79 @@ func (t *DurationTrackingTransport) RoundTrip(r *http.Request) (*http.Response,
}(start)
return http.DefaultTransport.RoundTrip(r)
}

// From go/src/net/http/httputil/reverseproxy.go

func singleJoiningSlash(a, b string) string {
aslash := strings.HasSuffix(a, "/")
bslash := strings.HasPrefix(b, "/")
switch {
case aslash && bslash:
return a + b[1:]
case !aslash && !bslash:
return a + "/" + b
}
return a + b
}

func joinURLPath(a, b *url.URL) (path, rawpath string) {
if a.RawPath == "" && b.RawPath == "" {
return singleJoiningSlash(a.Path, b.Path), ""
}
// Same as singleJoiningSlash, but uses EscapedPath to determine
// whether a slash should be added
apath := a.EscapedPath()
bpath := b.EscapedPath()

aslash := strings.HasSuffix(apath, "/")
bslash := strings.HasPrefix(bpath, "/")

switch {
case aslash && bslash:
return a.Path + b.Path[1:], apath + bpath[1:]
case !aslash && !bslash:
return a.Path + "/" + b.Path, apath + "/" + bpath
}
return a.Path + b.Path, apath + bpath
}

func newSingleHostReverseProxy(target *url.URL) *httputil.ReverseProxy {
targetQuery := target.RawQuery
director := func(req *http.Request) {
originalHost := req.Host

req.URL.Scheme = target.Scheme
req.URL.Host = target.Host
req.URL.Path, req.URL.RawPath = joinURLPath(target, req.URL)
if targetQuery == "" || req.URL.RawQuery == "" {
req.URL.RawQuery = targetQuery + req.URL.RawQuery
} else {
req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
}
req.Host = target.Host

if _, ok := req.Header["User-Agent"]; !ok {
// explicitly disable User-Agent so it's not set to default value
req.Header.Set("User-Agent", "")
}

// From https://github.com/golang/go/pull/36678
prior, ok := req.Header["X-Forwarded-Host"]
omit := ok && prior == nil // nil means don't populate the header
if !omit {
req.Header.Set("X-Forwarded-Host", originalHost)
}

prior, ok = req.Header["X-Forwarded-Proto"]
omit = ok && prior == nil // nil means don't populate the header
if !omit {
if req.TLS == nil {
req.Header.Set("X-Forwarded-Proto", "http")
} else {
req.Header.Set("X-Forwarded-Proto", "https")
}
}
// ReverseProxy will add X-Forwarded-For internally
}
return &httputil.ReverseProxy{Director: director}
}