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

URLs compromised under Windows #331

Closed
bneumann opened this issue Jan 7, 2017 · 14 comments
Closed

URLs compromised under Windows #331

bneumann opened this issue Jan 7, 2017 · 14 comments

Comments

@bneumann
Copy link
Contributor

bneumann commented Jan 7, 2017

Following workflow:

checkout helloworld git repo:
https://github.com/zettajs/zetta-hello-world

Build and run under Windows 10. Goto http://localhost:1337 Following json appears:

{ rel: [ "self", "edit" ], href: "http://localhost:1337/\servers\FirstName%20LastName\devices\9e967da5-bac2-4fff-b2ff-3541385ccf51" }, { rel: [ "http://rels.zettajs.io/type", "describedby" ], href: "http://localhost:1337/\servers\FirstName%20LastName/meta/state_machine" }, { title: "FirstName LastName", rel: [ "up", "http://rels.zettajs.io/server" ], href: "http://localhost:1337/\servers\FirstName%20LastName" }

If I run the same under linux I revceive correct URLS:

http://localhost:1337/servers/FirstName%20LastName

Problem is, that the REST API is broken without correct links and the Zetta Browser does not show anything

@mdobson
Copy link
Contributor

mdobson commented Jan 7, 2017

Hey @bneumann thanks for reporting this. What version of zetta are you using with that tutorial?

@mdobson
Copy link
Contributor

mdobson commented Jan 7, 2017

Hey @bneumann just looked at the repo you refer to, and it was using very old versions of zetta and the mock packages. I've updated them to latest. Can you let me know if that changes anything for you? I don't have a windows system to test with currently.

@bneumann
Copy link
Contributor Author

bneumann commented Jan 7, 2017

Hi,
I used the "latest" version of zetta in my package file. Should be 1.3.2 now. The console output seems to be working better now but the URLs are still not accessible

@mdobson
Copy link
Contributor

mdobson commented Jan 7, 2017

@AdamMagaluk Have access to a windows box? Would you be able to make a pass at a fix?

@bneumann
Copy link
Contributor Author

bneumann commented Jan 7, 2017

I would fix it if you could push me in the right direction. I browsed trhough the zetta code and did not find the correct file where the magic formatting happens

@kevinswiber
Copy link
Member

@AdamMagaluk @mdobson It would help to look for any path.join instances that alter the URL but don't include a replacement for Windows. From https://github.com/argo/argo-url-helper/blob/5e5cd602eee51651cb4b47c5d9208f213d8c4b75/url.js#L29:

parsed.pathname = path.join(parsed.pathname, pathname).replace(/\\/g, '/');

@mdobson
Copy link
Contributor

mdobson commented Jan 7, 2017

Sure @bneumann. I'd be more than happy to offer some guidance.

The API server underlying zetta is a package called argo. It manages all the http transactions, and does the nifty formatting stuff. It also has a concept of middleware similar to connect middleware in express.

The argo-url-helper module is where we do the url formatting.

As @kevinswiber mentioned above a good starting point is this line. https://github.com/argo/argo-url-helper/blob/master/url.js#L29

@kevinswiber
Copy link
Member

Here's a case where it will fail:

return protocol + '://' + path.join(host, ws.upgradeReq.url);

@bneumann
Copy link
Contributor Author

bneumann commented Jan 7, 2017

@kevinswiber Ah so I was already in the right spot. But tbh I was a bit confused what these modules do. But thanks to the explanation of @AdamMagaluk that it is a middlewary kind of thing I can take a deeper look into it. I let you know what I find out. Thanks for the tips!

@mdobson
Copy link
Contributor

mdobson commented Jan 7, 2017

Great! Let us know if you have any other questions.

@bneumann
Copy link
Contributor Author

bneumann commented Jan 7, 2017

Okay I made 2 pull requests on the two repositories. It is currently building, but it works now on my machine. Thanks guys!

AdamMagaluk pushed a commit that referenced this issue Jan 17, 2017
* Fixed replacement for windows URLs

* remove log entry
@AdamMagaluk
Copy link
Collaborator

Fixed with #332 new release coming.

@AdamMagaluk
Copy link
Collaborator

@bneumann It's fixed in [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants