Merge pull request #10206 from ckhsponge/fix/escape-path-id-on-simple…

…-rest-provider Fix for ids not being escaped in paths in the Simple Rest Data Provider
marmelab · Sep 23, 2024 · 9052a88 · 9052a88
2 parents 4ad9783 + 73d115f
commit 9052a88
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 6 deletions.
diff --git a/packages/ra-data-simple-rest/src/index.spec.ts b/packages/ra-data-simple-rest/src/index.spec.ts
@@ -68,6 +68,65 @@ describe('Data Simple REST Client', () => {
             expect(result.total).toEqual(42);
         });
     });
+    describe('getOne', () => {
+        it('should allow numeric id in path', async () => {
+            const httpClient = jest.fn().mockResolvedValue({ id: 123 });
+            const client = simpleClient('http://localhost:3000', httpClient);
+
+            await client.getOne('posts', { id: 123 });
+
+            expect(httpClient).toHaveBeenCalledWith(
+                'http://localhost:3000/posts/123',
+                expect.any(Object)
+            );
+        });
+        it('should escape id in path', async () => {
+            const httpClient = jest.fn().mockResolvedValue({ id: 'Post#123' });
+            const client = simpleClient('http://localhost:3000', httpClient);
+
+            await client.getOne('posts', { id: 'Post#123' });
+
+            expect(httpClient).toHaveBeenCalledWith(
+                'http://localhost:3000/posts/Post%23123',
+                expect.any(Object)
+            );
+        });
+    });
+    describe('update', () => {
+        it('should escape id in path', async () => {
+            const httpClient = jest.fn().mockResolvedValue({ id: 'Post#123' });
+            const client = simpleClient('http://localhost:3000', httpClient);
+
+            await client.update('posts', {
+                previousData: undefined,
+                id: 'Post#123',
+                data: { body: '' },
+            });
+
+            expect(httpClient).toHaveBeenCalledWith(
+                'http://localhost:3000/posts/Post%23123',
+                expect.any(Object)
+            );
+        });
+    });
+    describe('updateMany', () => {
+        it('should escape id in path', async () => {
+            const httpClient = jest
+                .fn()
+                .mockResolvedValue({ json: ['Post#123'] });
+            const client = simpleClient('http://localhost:3000', httpClient);
+
+            await client.updateMany('posts', {
+                data: { body: '' },
+                ids: ['Post#123'],
+            });
+
+            expect(httpClient).toHaveBeenCalledWith(
+                'http://localhost:3000/posts/Post%23123',
+                expect.any(Object)
+            );
+        });
+    });
     describe('delete', () => {
         it('should set the `Content-Type` header to `text/plain`', async () => {
             const httpClient = jest.fn().mockResolvedValue({ json: { id: 1 } });
@@ -89,10 +148,24 @@ describe('Data Simple REST Client', () => {
                 }
             );
         });
+        it('should escape id in path', async () => {
+            const httpClient = jest.fn().mockResolvedValue({ id: 'Post#123' });
+            const client = simpleClient('http://localhost:3000', httpClient);
+
+            await client.delete('posts', {
+                previousData: undefined,
+                id: 'Post#123',
+            });
+
+            expect(httpClient).toHaveBeenCalledWith(
+                'http://localhost:3000/posts/Post%23123',
+                expect.any(Object)
+            );
+        });
     });
     describe('deleteMany', () => {
         it('should set the `Content-Type` header to `text/plain`', async () => {
-            const httpClient = jest.fn().mockResolvedValue({ json: { id: 1 } });
+            const httpClient = jest.fn().mockResolvedValue({ json: [1] });
 
             const client = simpleClient('http://localhost:3000', httpClient);
 
@@ -120,5 +193,20 @@ describe('Data Simple REST Client', () => {
                 }
             );
         });
+        it('should escape id in path', async () => {
+            const httpClient = jest
+                .fn()
+                .mockResolvedValue({ json: ['Post#123'] });
+            const client = simpleClient('http://localhost:3000', httpClient);
+
+            await client.deleteMany('posts', {
+                ids: ['Post#123'],
+            });
+
+            expect(httpClient).toHaveBeenCalledWith(
+                'http://localhost:3000/posts/Post%23123',
+                expect.any(Object)
+            );
+        });
     });
 });
diff --git a/packages/ra-data-simple-rest/src/index.ts b/packages/ra-data-simple-rest/src/index.ts
@@ -83,7 +83,7 @@ export default (
     },
 
     getOne: (resource, params) =>
-        httpClient(`${apiUrl}/${resource}/${params.id}`, {
+        httpClient(`${apiUrl}/${resource}/${encodeURIComponent(params.id)}`, {
             signal: params?.signal,
         }).then(({ json }) => ({
             data: json,
@@ -147,7 +147,7 @@ export default (
     },
 
     update: (resource, params) =>
-        httpClient(`${apiUrl}/${resource}/${params.id}`, {
+        httpClient(`${apiUrl}/${resource}/${encodeURIComponent(params.id)}`, {
             method: 'PUT',
             body: JSON.stringify(params.data),
         }).then(({ json }) => ({ data: json })),
@@ -156,7 +156,7 @@ export default (
     updateMany: (resource, params) =>
         Promise.all(
             params.ids.map(id =>
-                httpClient(`${apiUrl}/${resource}/${id}`, {
+                httpClient(`${apiUrl}/${resource}/${encodeURIComponent(id)}`, {
                     method: 'PUT',
                     body: JSON.stringify(params.data),
                 })
@@ -172,7 +172,7 @@ export default (
         }).then(({ json }) => ({ data: json })),
 
     delete: (resource, params) =>
-        httpClient(`${apiUrl}/${resource}/${params.id}`, {
+        httpClient(`${apiUrl}/${resource}/${encodeURIComponent(params.id)}`, {
             method: 'DELETE',
             headers: new Headers({
                 'Content-Type': 'text/plain',
@@ -183,7 +183,7 @@ export default (
     deleteMany: (resource, params) =>
         Promise.all(
             params.ids.map(id =>
-                httpClient(`${apiUrl}/${resource}/${id}`, {
+                httpClient(`${apiUrl}/${resource}/${encodeURIComponent(id)}`, {
                     method: 'DELETE',
                     headers: new Headers({
                         'Content-Type': 'text/plain',