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

Inconsistent percent-encoding of keys #13187

Closed
rthill91 opened this issue Dec 9, 2020 · 7 comments
Closed

Inconsistent percent-encoding of keys #13187

rthill91 opened this issue Dec 9, 2020 · 7 comments
Labels
1 Bug 2 Fixed Resolution 3 UI ArangoDB Web Interface (frontend/Aardvark)
Milestone

Comments

@rthill91
Copy link

rthill91 commented Dec 9, 2020

My Environment

  • ArangoDB Version: 3.7.3
  • Storage Engine: RocksDB
  • Deployment Mode: Single Server
  • Deployment Strategy: Manual Start in Docker
  • Configuration: Default
  • Infrastructure: own
  • Operating System: Ubuntu 20.04
  • Total RAM in your machine: 32Gb
  • Disks in use: SSD
  • Used Package: Docker - official Docker library

Component, Query & Data

Affected feature:
AQL query using web interface

AQL query (if applicable):

insert
{
"_key": "test_%25"
}
into collection_name

Steps to reproduce

  1. Insert document containing percent-encodable key (e.g. test_%25)
  2. Attempt retrieval of the document in collection view or through rest api

Problem:
There is inconsistent percent-encoding happening. Documents are inserted correctly as given, but on retrieval are decoded.

I first noticed this problem when searching for a document by key in the collections view. The document was found, but when I click to open it up I get a DocumentNotFoundError. Manually fixing the key encoding in the URL results in the expected document (e.g. .../collection/collection_name/test_%2525)

Expected result:
Encoding (or lack-thereof) is consistent across insert and retrieval

@goedderz
Copy link
Member

Hi @rthill91,

that sound's correct to me. Percent-encoding (also known as URL encoding) is necessary only in the URL.

If you disagree, please describe in more detail what you're doing, what the result is, and what you find unexpected about the result.

@rthill91
Copy link
Author

The only aspect of this I think is an arangodb issue is what I mentioned about opening documents from the collection view of the web interface. A document with a key containing a substring that looks like a percent-encoded value will not open correctly.

@Simran-B
Copy link
Contributor

RETURN collection_name // [ [ { "_key": "test_%25" } ] ]

image

Clicking on the document results in:

image

Same error if you create an edge with key %25. The key is correctly set to %25 however.

It also displays the name incorrectly, and the _from/_to links are broken (same error as above):

image

@Simran-B Simran-B added 1 Bug 3 UI ArangoDB Web Interface (frontend/Aardvark) labels Dec 11, 2020
@Simran-B
Copy link
Contributor

Below function tries to decode the URL with the key not encoded first, and if it fails, it encodes the key. This test doesn't work because test%25 decodes to a valid URL test%:

js\apps\system_admin\aardvark\APP\frontend\js\views\documentsView.js

    goToDocument: function (error, data, msg) {
      if (error) {
        arangoHelper.arangoError('Error', msg.errorMessage);
      } else {
        window.modalView.hide();
        data = data.split('/');

        var url;
        try {
          url = 'collection/' + data[0] + '/' + data[1];
          decodeURI(url);
        } catch (ex) {
          url = 'collection/' + data[0] + '/' + encodeURIComponent(data[1]);
        }

        window.location.hash = url;
      }
    },

I'm not sure why it isn't encoded by default.

@goedderz
Copy link
Member

The only aspect of this I think is an arangodb issue is what I mentioned about opening documents from the collection view of the web interface. A document with a key containing a substring that looks like a percent-encoded value will not open correctly.

Ah, I see. That's not right, of course.

I'm not sure why it isn't encoded by default.

That looks strange indeed, and I'd expect it to be unconditionally encoded, too.

@Simran-B
Copy link
Contributor

This one is even weirder:

js\apps\system_admin\aardvark\APP\frontend\js\views\documentView.js:

      var navigateTo = $(e.target).attr('documentLink');
      var test = (navigateTo.split('%').length - 1) % 3;

      if (decodeURIComponent(navigateTo) !== navigateTo && test !== 0) {
        navigateTo = decodeURIComponent(navigateTo);
      }

      if (navigateTo) {
        window.App.navigate(navigateTo, {trigger: true});
      }

@Simran-B Simran-B added the 2 Fixed Resolution label Dec 6, 2022
@Simran-B Simran-B added this to the 3.9.0 milestone Dec 6, 2022
@Simran-B
Copy link
Contributor

Simran-B commented Dec 6, 2022

Should be fixed as of 3.9.0 by #14700

@Simran-B Simran-B closed this as completed Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 Bug 2 Fixed Resolution 3 UI ArangoDB Web Interface (frontend/Aardvark)
Projects
None yet
Development

No branches or pull requests

3 participants