From deb36c385d1092bfed2536fa219b182482ad5955 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 24 Sep 2024 15:25:42 -0400 Subject: [PATCH 1/6] start a query parameter e2e test --- spec/e2e/proxy_spec.rb | 5 +++++ spec/fixtures/extras/simple_origin_server.py | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/spec/e2e/proxy_spec.rb b/spec/e2e/proxy_spec.rb index ba04dfe0bb..eb6c8175d2 100644 --- a/spec/e2e/proxy_spec.rb +++ b/spec/e2e/proxy_spec.rb @@ -101,6 +101,11 @@ def browser expect(browser.div(id: 'test-div').present?).to be true end + it 'correctly passes query parameters to node URIs' do + browser.goto "#{ctr_base_url}/node/localhost/5001/one/with-query-params?artist=the beatles&album=let it be" + expect(browser.div(id: 'test-div').present?).to be true + end + it '/nginx/init needs a redirect' do url = "#{ctr_base_url}/nginx/init" browser.goto url diff --git a/spec/fixtures/extras/simple_origin_server.py b/spec/fixtures/extras/simple_origin_server.py index 2933de09ad..cf128bb2b6 100755 --- a/spec/fixtures/extras/simple_origin_server.py +++ b/spec/fixtures/extras/simple_origin_server.py @@ -21,6 +21,16 @@ def one_level_relative_redirect(): def one_level_down(): return "
A very simple page for testing
" +@app.route("/one/with-query-params") +def one_level_down(): + artist = request.args.get('artist') + album = request.args.get('album') + + if artist is None and album is None: + return 'You need to supply artist and album query parameters.', 404 + else: + return one_level_down(): + @app.route("/simple-redirect") def simple_redirect(): return redirect(url_for('app.simple_page'), code=302) From a80cb54ae5155e228bd8c6e48dac342dd30b2c15 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 24 Sep 2024 16:02:27 -0400 Subject: [PATCH 2/6] fix this proxy server --- spec/fixtures/extras/simple_origin_server.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/fixtures/extras/simple_origin_server.py b/spec/fixtures/extras/simple_origin_server.py index cf128bb2b6..0b18695ec4 100755 --- a/spec/fixtures/extras/simple_origin_server.py +++ b/spec/fixtures/extras/simple_origin_server.py @@ -22,14 +22,15 @@ def one_level_down(): return "
A very simple page for testing
" @app.route("/one/with-query-params") -def one_level_down(): +def one_level_down_with_query_params(): artist = request.args.get('artist') album = request.args.get('album') + print("artist: {} album: {}".format(artist, album)) - if artist is None and album is None: + if artist is None or album is None: return 'You need to supply artist and album query parameters.', 404 else: - return one_level_down(): + return one_level_down() @app.route("/simple-redirect") def simple_redirect(): From 535a2874f83b817a77181edf2ea7c2fb274c83cf Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 24 Sep 2024 16:05:00 -0400 Subject: [PATCH 3/6] add tests for both node and rnode --- spec/e2e/proxy_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/e2e/proxy_spec.rb b/spec/e2e/proxy_spec.rb index eb6c8175d2..10f5114abc 100644 --- a/spec/e2e/proxy_spec.rb +++ b/spec/e2e/proxy_spec.rb @@ -102,7 +102,16 @@ def browser end it 'correctly passes query parameters to node URIs' do - browser.goto "#{ctr_base_url}/node/localhost/5001/one/with-query-params?artist=the beatles&album=let it be" + url = "#{ctr_base_url}/node/localhost/5001/one/with-query-params?artist=the beatles&album=let it be" + browser.goto(url) + expect(browser.url).to eq(url) + expect(browser.div(id: 'test-div').present?).to be true + end + + it 'correctly passes query parameters to rnode URIs' do + url = "#{ctr_base_url}/rnode/localhost/5000/one/with-query-params?artist=the beatles&album=let it be" + browser.goto(url) + expect(browser.url).to eq(url) expect(browser.div(id: 'test-div').present?).to be true end From 442b5b476780b85efe49b4730208c2746a237f8d Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 24 Sep 2024 16:26:48 -0400 Subject: [PATCH 4/6] should be url encoded --- spec/e2e/proxy_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/e2e/proxy_spec.rb b/spec/e2e/proxy_spec.rb index 10f5114abc..ff08cf3480 100644 --- a/spec/e2e/proxy_spec.rb +++ b/spec/e2e/proxy_spec.rb @@ -102,14 +102,14 @@ def browser end it 'correctly passes query parameters to node URIs' do - url = "#{ctr_base_url}/node/localhost/5001/one/with-query-params?artist=the beatles&album=let it be" + url = "#{ctr_base_url}/node/localhost/5001/one/with-query-params?artist=the%20beatles&album=let%20it%20be" browser.goto(url) expect(browser.url).to eq(url) expect(browser.div(id: 'test-div').present?).to be true end it 'correctly passes query parameters to rnode URIs' do - url = "#{ctr_base_url}/rnode/localhost/5000/one/with-query-params?artist=the beatles&album=let it be" + url = "#{ctr_base_url}/rnode/localhost/5000/one/with-query-params?artist=the%20beatles&album=let%20it%20be" browser.goto(url) expect(browser.url).to eq(url) expect(browser.div(id: 'test-div').present?).to be true From b88d45f8630e53056438aba89c37a522bc8ad909 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Wed, 25 Sep 2024 09:29:59 -0400 Subject: [PATCH 5/6] set the uri to only the uri, disregarding query params --- mod_ood_proxy/lib/node_proxy.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod_ood_proxy/lib/node_proxy.lua b/mod_ood_proxy/lib/node_proxy.lua index 705d6f86d9..02ec1806f8 100644 --- a/mod_ood_proxy/lib/node_proxy.lua +++ b/mod_ood_proxy/lib/node_proxy.lua @@ -34,11 +34,11 @@ function node_proxy_handler(r) local conn = {} conn.user = user conn.server = host .. ":" .. port - conn.uri = uri and (r.args and (uri .. "?" .. r.args) or uri) or r.uri + conn.uri = uri or r.uri or '/' -- last ditch effort to ensure that the uri is at least something -- because the request-line of an HTTP request _has_ to have something for a URL - if conn.uri == nil or conn.uri == '' then + if conn.uri == '' then conn.uri = '/' end From 5251ac8ce6c423c7af8cbc62ee692a328c818fdb Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 3 Oct 2024 10:10:01 -0400 Subject: [PATCH 6/6] reset node_proxy.lua back to original --- mod_ood_proxy/lib/node_proxy.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod_ood_proxy/lib/node_proxy.lua b/mod_ood_proxy/lib/node_proxy.lua index 02ec1806f8..705d6f86d9 100644 --- a/mod_ood_proxy/lib/node_proxy.lua +++ b/mod_ood_proxy/lib/node_proxy.lua @@ -34,11 +34,11 @@ function node_proxy_handler(r) local conn = {} conn.user = user conn.server = host .. ":" .. port - conn.uri = uri or r.uri or '/' + conn.uri = uri and (r.args and (uri .. "?" .. r.args) or uri) or r.uri -- last ditch effort to ensure that the uri is at least something -- because the request-line of an HTTP request _has_ to have something for a URL - if conn.uri == '' then + if conn.uri == nil or conn.uri == '' then conn.uri = '/' end