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

String ID with non-URLsafe chars causes 404 when viewing detail page #1041

Open
RichardBradley opened this issue Apr 19, 2016 · 10 comments
Open
Labels

Comments

@RichardBradley
Copy link
Contributor

I have an entity with a string ID field.
If the ID contains non-URLsafe chars like "+" or "/" then the detail links from the list view correctly escapes these chars, i.e. https://my-app.example.com/#/users/edit/x5ig%2BnkzATc%252FCBz , however following that link does not work:

I believe this is because the REST GET request for the user does not correctly escape the ID when using it as a URL parameter:

ng-admin.min.js:25 GET https://my-app.example.com/api/users/x5ig+nkzATc/CBz 404 (Not Found)
@sam2x
Copy link

sam2x commented May 19, 2016

It's not an ng-admin issue, and it's the default behaviour of restangular and $http (mgonto/restangular#232)

You need to safe url-encode your id first. Here an example : http://plnkr.co/edit/qrw9Y8rfUUmHM4rOJo1q?p=preview (the user John has 'test+/' as id.

I just used a Restangular interceptor and encodeURIComponent the url.

@RichardBradley
Copy link
Contributor Author

Yes, this is a bug in restangular. I have commented on that issue which was incorrectly closed.

I think that ng-admin should consider adding the workaround you suggest to its default configuration until the underlying bug in restangular is fixed.

In the meantime, I'll add that workaround to my app.

Thanks for your help.

@RichardBradley
Copy link
Contributor Author

Your workaround doesn't work; that plunker still sends invalid requests to the backend:

@http: GET /api/users/test+/

However, the "backend.js" has a broken URL parser which accepts these invalid URLs:

var _getModelId = function(url){
  var regex =  /^\/api\/([a-z]+)\/?([a-z0-9\+\/]+)?/g;

A similar example with a real REST backend doesn't work.
I'll try to create a better workaround.

@RichardBradley
Copy link
Contributor Author

I don't think this can be fixed with a request interceptor.

In general, how could a request interceptor distinguish between a "get" request for a "foo" object with id "1/bar/2" or a "get" request for a "bar" object with id "2" which is a child of "foo" with id "1"?

@sam2x
Copy link

sam2x commented May 22, 2016

Indeed, your case can only work if you dont have two object to handle, eg with nodejs:

  router.get('/foo/:id')

Will works. But as soon you have two entity to handle, it's not gonna works, eg:

router.get('/foo/:id/bar/:pid')

I can't help you here, the only way to solve cleanly this problem is to either change the id generator used to avoid conflict or future edge-case, or use a correct encoder on each parameter.

@RichardBradley
Copy link
Contributor Author

This isn't a Restangular bug; it's an ng-admin bug.
The broken code is Application.getRouteFor in admin-config:

getRouteFor(entity, viewUrl, viewType, identifierValue, identifierName) {
        let baseApiUrl = entity.baseApiUrl() || this.baseApiUrl(),
            url = viewUrl || entity.getUrl(viewType, identifierValue, identifierName);

        // If the view or the entity don't define the url, retrieve it from the baseURL of the entity or the app
        if (!url) {
            url = baseApiUrl + entity.name();
            if (identifierValue != null) {
                url += '/' + identifierValue;
            }
        } else if (!/^(?:[a-z]+:)?\/\//.test(url)) {
            // Add baseUrl for relative URL
            url = baseApiUrl + url;
        }

        return url;
    }

I'll look into submitting a PR, and I'll update those Restangular bugs

@sam2x
Copy link

sam2x commented May 23, 2016

Hmmm, if it's entityId, have you try to use the url hook which let you define the way you send ng-admin entityId ?

 nga.entity('users')
    .url(function(entity, type, entityId){
      return entity + (entityId ? '/' + encodeURIComponent(entityId) : '');
 }) 

ps: You were right, my initial "workaround" was incorrect due to regex 'catcher', the full url was url-encoded, which was wrong.

@RichardBradley
Copy link
Contributor Author

have you try to use the url hook which let you define the way you send ng-admin entityId

That would fix it, yes.

As I'm already running a fork of ng-admin due to several open bugs (see my PRs), I'll just patch the bug at source and submit a PR with a proper fix.

@fzaninotto
Copy link
Member

Fixed by #1088

@RichardBradley
Copy link
Contributor Author

This still doesn't work for me.
The Restangular request is fixed, if you jump directly to the entity Edit view in question.

However, the List view detail link for an entity with a non-URL-safe id is broken, as the link is escaped with "~" instead of "%".

You can see this in the plunkr linked at the top of this thread: http://plnkr.co/edit/qrw9Y8rfUUmHM4rOJo1q?p=preview

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

No branches or pull requests

4 participants