-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Include URI query string in proxied requests #1971
Include URI query string in proxied requests #1971
Conversation
A wild justinsteven appears. Strange. I'm pretty sure this used to work. I wonder when/how this stopped working... I'll take a look. |
I tried proxing a request on an old version of Firefox and the query string was sent ok. But I encountered a different bug. The response body is never returned, regardless of whether the destination page was cross-origin. Tried on old Chrome and old Firefox, with and without this patch. I don't see any immediate harm in including this patch, but the proxy needs further review and testing (by someone who isn't me). |
I'll try and spend some time this evening having a look. |
@bcoles o/ You said:
Interesting. It's definitely broken for me. I'll get to that soon.
I don't think I've seen that. I'll keep an eye out. Oh, that is, unless beef is throwing
+1 I'm using proxy in the lab right now for a POC, but nothing I'd call real-world use. Someone who knows the guts of it should definitely take a look. The brokennnessI've done varying tests with the hooked URL being http and https; the proxy destination URL being http and https; proxy clients being Firefox and curl. No luck on any account - the query string is never included by the hooked browser. I've unwound my patch and the crazy reverse proxy setup I was using and it still seems buggy to me. Ruby:
My environment has the locale fix per #1290 (comment) I'm at commit % git diff
diff --git a/beef b/beef
index 1c85abab..569b995b 100755
--- a/beef
+++ b/beef
@@ -166,7 +166,8 @@ end
# Connect to DB
ActiveRecord::Base.logger = nil
OTR::ActiveRecord.migrations_paths = [File.join('core', 'main', 'ar-migrations')]
-OTR::ActiveRecord.configure_from_hash!(adapter:'sqlite3', database:db_file)
+#OTR::ActiveRecord.configure_from_hash!(adapter:'sqlite3', database:db_file)
+OTR::ActiveRecord.configure_from_hash!(adapter:'sqlite3', database:db_file, pool:250)
# Migrate (if required)
context = ActiveRecord::Migration.new.migration_context
if context.needs_migration?
diff --git a/config.yaml b/config.yaml
index d4d93e81..4ca906a2 100644
--- a/config.yaml
+++ b/config.yaml
@@ -8,7 +8,7 @@
beef:
version: '0.5.0.0-alpha-pre'
# More verbose messages (server-side)
- debug: false
+ debug: true
# More verbose messages (client-side)
client_debug: false
# Used for generating secure tokens
@@ -18,15 +18,15 @@ beef:
# Used by both the RESTful API and the Admin interface
credentials:
user: "beef"
- passwd: "beef"
+ passwd: "<SNIP>"
# Interface / IP restrictions
restrictions:
# subnet of IP addresses that can hook to the framework
permitted_hooking_subnet: ["0.0.0.0/0", "::/0"]
# subnet of IP addresses that can connect to the admin UI
- #permitted_ui_subnet: ["127.0.0.1/32", "::1/128"]
- permitted_ui_subnet: ["0.0.0.0/0", "::/0"]
+ permitted_ui_subnet: ["127.0.0.1/32", "::1/128"]
+ #permitted_ui_subnet: ["0.0.0.0/0", "::/0"]
# slow API calls to 1 every api_attempt_delay seconds
api_attempt_delay: "0.05" Regarding the ActiveRecord
I don't know why I'm getting this error
But I got sick of seeing the error (and having beef go sideways until I bounced it) so setting the Back to the proxy. With the hooked URL of http://127.0.0.2:3000/demos/basic.html, Proxy with a query string is definitely broken for same-site and cross-site requests beef debug:
Use the proxy cross-site:
beef debug (Filtering for just the proxy stuff related to the above request):
(I don't know why beef is complaining about "Proxy browser not set", I've definitely set it) If I watch the hooked browser's F12 "Network" tab, the request to http://1.1.1.1/path is sent without the querystring Use the proxy same-site:
beef debug (Filtering for just the proxy stuff related to the above request):
If I watch the hooked browser's F12 "Network" tab, the request to http://127.0.0.2:3000/path is sent without the querystring |
Oh right... the proxy. Not the Rider/Requester. That would explain it. Confirmed. Without this patch, the URL query string is not sent. With the patch it is sent. Edit: The issue related to the response body not showing the Proxy history tab exists before and after this patch. Edit 2: The proxy and what used to be called the Rider/Requester are very similar in the UI, and in the server-side code, and in the client side hook API. The code should probably be merged into one extension (called "proxy"), with the option to switch the local proxy server (on port 6789) on or off in the config. |
😅 👍 That'll teach me to write a brief PR message Glad we're on the same page. And yep, I know what you mean now, the Proxy history tab is broken for me too. |
Category
Bug
Feature/Issue Description
Q: Please give a brief summary of your feature/fix
A: Proxied requests with query string params (e.g. https://foo.example/bar?fizz=buzz) were being sent without the query string (i.e. https://foo.example/bar)
Q: Give a technical rundown of what you have changed (if applicable)
A: If the
URI
has aquery
then construct a path that includes the query stringTest Cases
Q: Describe your test cases, what you have covered and if there are any use cases that still need addressing.
A:
No test cases written. I've done simple testing in the lab only. I'd appreciate a careful look and further testing.
✔️ Proxied requests without a query string work, and are sent without a query string
✔️ Proxied requests with a query string (
?foo
) work, and are sent with the correct query string✔️ Proxied requests with a query string (
?foo=bar
) work, and are sent with the correct query stringWiki Page
If you are adding a new feature that is not easily understood without context, please draft a section to be added to the Wiki below.
N/A