Sandpack / nodebox-runtime

Nodebox is a runtime for executing Node.js modules in the browser.
https://sandpack.codesandbox.io/docs/advanced-usage/nodebox
Other
727 stars 41 forks source link

Use "hostname" instead of "host" in `worker.js` #40

Closed brianjenkins94 closed 1 year ago

brianjenkins94 commented 1 year ago
// _0x1f31f9(0x1029) -> "host"
this[_0x1f31f9(0x1144)] = (_0x47c213[_0x1f31f9(0x10ac)] || _0x1f31f9(0xc5d)) + '//' + _0x47c213[_0x1f31f9(0x1029)] + (_0x47c213[_0x1f31f9(0xb50)] ? ':' + _0x47c213[_0x1f31f9(0xb50)] : '') + (_0x47c213[_0x1f31f9(0x2a8)] || '/'),

should probably be:

this[_0x1f31f9(0x1144)] = (_0x47c213[_0x1f31f9(0x10ac)] || _0x1f31f9(0xc5d)) + '//' + _0x47c213["hostname"] + (_0x47c213[_0x1f31f9(0xb50)] ? ':' + _0x47c213[_0x1f31f9(0xb50)] : '') + (_0x47c213[_0x1f31f9(0x2a8)] || '/')

Otherwise you can build invalid URLs like: http://hostname:8080:8080/pathname

DeMoorJasper commented 1 year ago

Fixed internally, will deploy once tests pass, thanks for reporting this issue. Doesn't seem like it's part of any critical path though as hostname does not exist in Node.js this only applies to some error reporting logic

brianjenkins94 commented 1 year ago

It doesn't appear that this has been fixed.

DeMoorJasper commented 1 year ago

It doesn't appear that this has been fixed.

It hasn't because it wasn't a bug, can you share a repro of an actual issue related to this?