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

Fix bug with finding nodes in index with spaces in value. #57

Merged
merged 2 commits into from
Sep 6, 2012

Conversation

rdvdijk
Copy link
Collaborator

@rdvdijk rdvdijk commented Sep 6, 2012

Setup:

new_node = @neo.create_node
key = "someKey"
value = "some value" # this has a space
@neo.add_node_to_index("indexName", key, value, new_node)

Before fix:

found_node = @neo.find_node_index("indexName", key, value)
# => nil

After fix:

found_node = @neo.find_node_index("indexName", key, value)
# => [{ .. a whole lot of JSON .. }]

@rdvdijk
Copy link
Collaborator Author

rdvdijk commented Sep 6, 2012

One more thing: I also can't search for values with a slash (/) in it.. This pull request does not fix that.

@rdvdijk
Copy link
Collaborator Author

rdvdijk commented Sep 6, 2012

Added another fix, searching is now possible for both spaces and slashes.

@rdvdijk
Copy link
Collaborator Author

rdvdijk commented Sep 6, 2012

Finding now works, but get_node_index is still broken. See the following test:

it "can get a node index with a slash in the value" do
  new_node = @neo.create_node
  key = generate_text(6)
  value = generate_text + "/" + generate_text
  @neo.add_node_to_index("test_node_index", key, value, new_node)
  new_index = @neo.get_node_index("test_node_index", key, value)
  new_index.should_not be_nil
  @neo.remove_node_from_index("test_node_index", key, value, new_node)
end

This is because the slash (/) is not escaped in def get (URL.encode does not escape slashes, and rightly so), you will get requests like this:

http://localhost:7474/db/data/index/node/test_node_index/ynvzuy/gwmurapb/rchsbnwp

This will result in a 'Not found' reply.

The way to fix this is to escape the slash with %2F.

http://localhost:7474/db/data/index/node/test_node_index/ynvzuy/gwmurapb%2Frchsbnwp

However, this is not so easy to fix. If we escape slashes outside of def get, the values would get double-encoded to %252F (%2F url-encoded is %252F).

Any ideas how to fix this elegantly?

maxdemarzi added a commit that referenced this pull request Sep 6, 2012
Fix bug with finding nodes in index with spaces in value.
@maxdemarzi maxdemarzi merged commit 8c115ca into maxdemarzi:master Sep 6, 2012
@maxdemarzi
Copy link
Owner

How critical is this? I don't mind putting in a hack if necessary. The workaround is to not use slashes in the value of the indexed key, (escape with -slash-, or whatever). Where do you see this being used?

@rdvdijk
Copy link
Collaborator Author

rdvdijk commented Sep 6, 2012

This happened in the application we are developing, and we have fixed it in the way you proposed. No need to fix it now, but I do think it is a bug.

willkessler pushed a commit to willkessler/neography that referenced this pull request Apr 21, 2014
deja should check in composed_attributes too + composed_attributes shoul...
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