Code review comment for ~ahasenack/ubuntu/+source/node-configurable-http-proxy:kinetic-fix-build-1967024

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the MP, Andreas.

Hm, I can't make the build succeed with your patch. I could reproduce the problem locally, and when I apply the changes you suggested I do indeed notice that there are no more problems with the proxy blocking requests, but I'm seeing other errors here:

+ jasmine JASMINE_CONFIG_PATH=test/jasmine.json
Randomized with seed 01637
Started
.......................................00:59:00.990 [ConfigProxy] info: Adding route / -> http://127.0.0.1:9001
00:59:00.990 [ConfigProxy] info: Route added / -> http://127.0.0.1:9001
.F00:59:11.063 [ConfigProxy] error: 503 GET /missing/ws connect ECONNREFUSED 127.0.0.1:54321
F00:59:11.072 [ConfigProxy] error: 500 GET /% URIError: URI malformed
    at decodeURIComponent (<anonymous>)
    at ConfigurableProxy.targetForReq (/home/ubuntu/node-configurable-http-proxy/node-configurable-http-proxy-4.5.0+~cs15.1.4/lib/configproxy.js:385:27)
    at ConfigurableProxy.handleProxy (/home/ubuntu/node-configurable-http-proxy/node-configurable-http-proxy-4.5.0+~cs15.1.4/lib/configproxy.js:529:17)
    at ConfigurableProxy.handleProxyWeb (/home/ubuntu/node-configurable-http-proxy/node-configurable-http-proxy-4.5.0+~cs15.1.4/lib/configproxy.js:613:17)
    at Server.<anonymous> (/home/ubuntu/node-configurable-http-proxy/node-configurable-http-proxy-4.5.0+~cs15.1.4/lib/configproxy.js:198:27)
    at Server.emit (events.js:314:20)
    at parserOnIncoming (_http_server.js:779:12)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:122:17)
....F......00:59:21.149 [ConfigProxy] error: 404 GET /foo/bar
.F...

Failures:
1) Proxy Tests hostRouting: routes by host
  Message:
    Unhandled promise rejection: RequestError: Error: getaddrinfo ENOTFOUND test.localhost.jovyan.org
  Stack:
    error properties: null({ cause: Error: getaddrinfo ENOTFOUND test.localhost.jovyan.org, error: Error: getaddrinfo ENOTFOUND test.localhost.jovyan.org, options: Object({ method: 'GET',
url: 'http://127.0.0.1:8902', followRedirect: false, uri: 'http://test.localhost.jovyan.org:8902/some/path', callback: Function, transform: undefined, simple: true, resolveWithFullResponse
: false, transform2xxOnly: false }), response: undefined, constructor: Function })
        at new RequestError (/home/ubuntu/node-configurable-http-proxy/node-configurable-http-proxy-4.5.0+~cs15.1.4/debian/tests/test_modules/request-promise-core/lib/errors.js:14:15)
        at Request.plumbing.callback (/home/ubuntu/node-configurable-http-proxy/node-configurable-http-proxy-4.5.0+~cs15.1.4/debian/tests/test_modules/request-promise-core/lib/plumbing.js:
87:29)
.....
(several other errors here)

/me investigates...

OK, I found something interesting. Here's what I see when I use dig to determine the IP of test.localhost.jovyan.org using my default DNS server:

$ dig test.localhost.jovyan.org

; <<>> DiG 9.16.1-Ubuntu <<>> test.localhost.jovyan.org
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 44452
;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 65494
;; QUESTION SECTION:
;test.localhost.jovyan.org. IN A

;; Query time: 35 msec
;; SERVER: 127.0.0.53#53(127.0.0.53)
;; WHEN: Sat Apr 30 01:08:39 EDT 2022
;; MSG SIZE rcvd: 54

No deal. However, here's what I get when I use Google's DNS server:

$ dig @8.8.8.8 test.localhost.jovyan.org

; <<>> DiG 9.16.1-Ubuntu <<>> @8.8.8.8 test.localhost.jovyan.org
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 35195
;; flags: qr rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 512
;; QUESTION SECTION:
;test.localhost.jovyan.org. IN A

;; ANSWER SECTION:
test.localhost.jovyan.org. 300 IN A 127.0.0.1

;; Query time: 19 msec
;; SERVER: 8.8.8.8#53(8.8.8.8)
;; WHEN: Sat Apr 30 01:11:18 EDT 2022
;; MSG SIZE rcvd: 70

Strange. /me investigates more...

Ah, found it. My router enables DNS Rebinding protection by default, which filters out local addresses from DNS answers. I disabled it and now the build passes as expected.

As you said, I believe this is useful for Debian and, as it happens, I'm part of the JavaScript team there. I think it makes more sense to accept your patch there and upload a new version of the package with the fix than to introduce this delta, but in any case I'm approving this MP so feel free to upload it as is.

+1

review: Approve

« Back to merge proposal