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

Update route.js to support "put" more than 2 levels deep #28

Merged
merged 1 commit into from
May 15, 2014

Conversation

VARG0S
Copy link
Contributor

@VARG0S VARG0S commented May 9, 2014

Modified routes.js to support deeper "levels"
was having trouble with the code in router.put binding to an $id more than 2 levels deep

while (o && pos.length - 1) {
  o = o[pos.shift()]
}

Example:
Using the example from the readme, posting to

http://localhost:3000/rest/blogpost/$id/comments/ 

will work fine, however if you add another level like

http://localhost:3000/rest/blogpost/$id/comments/$id/notes/

then an error would occur.

I modified the code to assume that any additional layers would follow a path of

/level1/$id/level2/$id/level3/...etc

Modify router.put to support deeper "levels" 
was having trouble with 

```javascript

while (o && pos.length - 1) {
  o = o[pos.shift()]
}

```

binding to an $id

Example:
Using the example from the readme, posting to

http://localhost:3000/rest/blogpost/$id/comments/ 
will work fine, however if you add another level like

http://localhost:3000/rest/blogpost/$id/comments/$id/notes/

I modified the code to assume that any additional layers would follow a path of

/level1/$id/level2/$id/level3/...etc
@jspears
Copy link
Owner

jspears commented May 14, 2014

Thanks for the patch... Not real crazy about using eval for this, especially since they are untrusted strings, it would not be hard to create a vulenerability. I'll rework the walk stuff to do the right thing.

@VARG0S
Copy link
Contributor Author

VARG0S commented May 14, 2014

Sounds good, we weren't too crazy about the eval either, but it sufficed for a demo we are working on. Thanks!

@jspears jspears merged commit 2b6a8be into jspears:master May 15, 2014
@jspears
Copy link
Owner

jspears commented May 15, 2014

I think I have some fixes in there for you. If you have issues let me
know, otherwise I will do a release in a day or two.

On Wed, May 14, 2014 at 12:43 PM, Matt notifications@github.com wrote:

Sounds good, we weren't too crazy about the eval either, but it sufficed
for a demo we are working on. Thanks!


Reply to this email directly or view it on GitHubhttps://github.com//pull/28#issuecomment-43105714
.

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

Successfully merging this pull request may close these issues.

2 participants