feat(utils/body): add dot notation support for parseBody

[fzn0x]

Closes #2656

cc @yusukebe @MathurAditya724

The author should do the following, if applicable

• Add tests
• Run tests
• bun denoify to generate files for Deno
• bun run format:fix && bun run lint:fix to format the code

[MathurAditya724]

These are some basic changes you can do to improve this. I haven't looked at the setNestedValue func and the test cases (this was all the time I had 😅), but I will try to look at it soon.

[fzn0x]

@MathurAditya724 Reviews resolved, to decide whether is it dot or transformDotNotation naming, we can wait @yusukebe first.

[fzn0x]

Dot notation support documentation will be added here: https://hono.dev/api/request#parsebody

---

Dot Notation

// assume you pass `obj.key1` in the request body
const body = await c.req.parseBody({ UNDECIDED: true })
body['obj'] // { key1: 'value' }

body['obj'] is a nested object.

All fields key with dot notation (e.g 'obj.key1.label' or 'obj.key1') will be parsed to nested objects.

[MathurAditya724]

Yes, you will have to create a separate PR for that at https://github.com/honojs/website

[yusukebe]

Woooow. Super fast and quick! I'll review this now.

[fzn0x]

Thanks @yusukebe I will run denoify and lint once again

[MathurAditya724]

Just some minor changes for the improvements you did based on my review

[yusukebe]

Hey. The following test does not pass now. But is this expected? What do you think? @fzn0x @MathurAditya724

it('should parse data if both `all` and `dot` are set', async () => {
  const data = new FormData()
  data.append('obj.sub.foo', 'value1')
  data.append('obj.sub.foo', 'value2')
  data.append('key', 'value3')

  const req = createRequest(FORM_URL, 'POST', data)

  expect(await parseBody(req, { dot: true, all: true })).toEqual({
    obj: { sub: { foo: ['value1', 'value2'] } }, // Is tis expected??
    key: 'value3',
  })
})

Test test result:

- Expected
+ Received

  Object {
    "key": "value3",
    "obj": Object {
      "sub": Object {
-       "foo": Array [
-         "value1",
-         "value2",
-       ],
+       "foo": "value2",
      },
    },
  }

[fzn0x]

This is unexpected, let me fix that @yusukebe

[yusukebe]

Anyway, this is a new "feature," so it may not be merged into the main immediately. So you don't have to hurry up:)

[vanodevium]

@yusukebe

I need it for everyday tasks :)

[MathurAditya724]

@fzn0x I believe handleNestedValues and handleParsingAllValues might be clashing. Do let me know if you require any help

[fzn0x]

Looks like

        if (
          !nestedForm[key] ||
          typeof nestedForm[key] !== 'object' ||
          Array.isArray(nestedForm[key])
        ) {
          nestedForm[key] = nestedForm[key] || {}
        }

This code only return the last value of the array @MathurAditya724 I will try to fix it and hope get back with good news

[MathurAditya724]

Interesting, let me check

[MathurAditya724]

Can I make changes to this PR? Or should I fork your repo?

[fzn0x]

I found the solution but the hard thing is to keep the code clean 😅 @MathurAditya724 @yusukebe

I guess we can continue with the review 🔥

[fzn0x]

I believe we can add this in v4.4, what do you think? @yusukebe

[yusukebe]

I believe we can add this in v4.4, what do you think? @yusukebe

Yes. Let's include this in v4.4.

[usualoma]

Hi @fzn0x!
Thank you for the great PR.
I have two comments.

all is always enabled.

Since all is always enabled in the current branch, I think the code needs to be modified by adding the following test.

diff --git a/src/utils/body.test.ts b/src/utils/body.test.ts
index cc154ef4..f9f7e3c7 100644
--- a/src/utils/body.test.ts
+++ b/src/utils/body.test.ts
@@ -50,6 +50,20 @@ describe('Parse Body Util', () => {
     })
   })
 
+  it('should override value if `all` option is false', async () => {
+    const data = new FormData()
+    data.append('file', 'aaa')
+    data.append('file', 'bbb')
+    data.append('message', 'hello')
+
+    const req = createRequest(FORM_URL, 'POST', data)
+
+    expect(await parseBody(req)).toEqual({
+      file: 'bbb',
+      message: 'hello',
+    })
+  })
+
   it('should parse multiple values if `all` option is true', async () => {
     const data = new FormData()
     data.append('file', 'aaa')

Objects containing arbitrary data should be initialized using Object.create(null)

This would also require modification of existing code, but for objects that receive a request and put in arbitrary data, they should be initialized using Object.create(null).

diff --git a/src/utils/body.ts b/src/utils/body.ts
index a3382806..8ef63893 100644
--- a/src/utils/body.ts
+++ b/src/utils/body.ts
@@ -72,7 +72,7 @@ function convertFormDataToBodyData<T extends BodyData = BodyData>(
   formData: FormData,
   options: ParseBodyOptions
 ): T {
-  const form: BodyData = {}
+  const form: BodyData = Object.create(null)
 
   formData.forEach((value, key) => {
     const shouldParseAllValues = options.all || key.endsWith('[]')
@@ -140,7 +140,7 @@ const handleNestedValues = (
           typeof nestedForm[key] !== 'object' ||
           Array.isArray(nestedForm[key])
         ) {
-          nestedForm[key] = {}
+          nestedForm[key] = Object.create(null)
         }
         nestedForm = nestedForm[key] as BodyData
       }

[fzn0x]

Fix added! kindly review, thanks @usualoma

[usualoma]

@fzn0x Thank you! > 716de84

Sorry to add one more point. From a security standpoint, I think it would be better to prevent the File object's properties from being able to be updated.

diff --git a/src/utils/body.test.ts b/src/utils/body.test.ts
index 41235292..fb6da697 100644
--- a/src/utils/body.test.ts
+++ b/src/utils/body.test.ts
@@ -191,6 +191,32 @@ describe('Parse Body Util', () => {
     })
   })
 
+  it('should not update file object properties', async () => {
+    const file = new File(['foo'], 'file1', {
+      type: 'application/octet-stream',
+    })
+    const data = new FormData()
+
+    const req = createRequest(FORM_URL, 'POST', data)
+    vi.spyOn(req, 'formData').mockImplementation(
+      async () =>
+        ({
+          forEach: (cb) => {
+            cb(file, 'file', data)
+            cb('hoo', 'file.hoo', data)
+          },
+        } as FormData)
+    )
+
+    const parsedData = await parseBody(req, { dot: true })
+    expect(parsedData.file).not.instanceOf(File)
+    expect(parsedData).toEqual({
+      file: {
+        hoo: 'hoo',
+      },
+    })
+  })
+
   it('should parse multiple values if key ends with `[]`', async () => {
     const data = new FormData()
     data.append('file[]', 'aaa')
diff --git a/src/utils/body.ts b/src/utils/body.ts
index 1c77ab98..f07c6433 100644
--- a/src/utils/body.ts
+++ b/src/utils/body.ts
@@ -138,7 +138,8 @@ const handleNestedValues = (
         if (
           !nestedForm[key] ||
           typeof nestedForm[key] !== 'object' ||
-          Array.isArray(nestedForm[key])
+          Array.isArray(nestedForm[key]) ||
+          nestedForm[key] instanceof File
         ) {
           nestedForm[key] = Object.create(null)
         }

[MathurAditya724]

Hey @fzn0x, I have gone through the functions and made some improvements. All the tests are passing. Also, I added some pointers where we can improve this more. Let me know what you think

[fzn0x]

Thanks for the reviews,
I will submit a/some PR/s for the review above.

[fzn0x]

Adding tsdoc for better developer experience

[yusukebe]

LGTM!

[yusukebe]

Thanks y'all. Now ready to merge to the next branch for the v4.4.0.

[iveshenry18]

Was about to implement this myself when I found this—awesome work!

Curious—has there been any discussion around implementing the inverse (a flattenObject function) for the Hono client? It's relatively simple for the user to implement, but it would be nice to have a Hono-supported implementation that's guaranteed to be kept in sync (and losslessly round-trip) with this implementation. Would be awesome in terms of giving Hono even more of an RPC feel.

[yusukebe]

Hi @iveshenry18 !

When we were challenged to create a function like a JSON path in our validator, we used a function similar to "flattenObject." But we decided not to make it. We will not implement it as a utility function if it does not require any features.

[yusukebe]

@iveshenry18

Oops! I just checked and it was not a flattenObject, but a function that parses a dot-notation.

[iveshenry18]

Interesting—that's essentially what the code in this PR does.

I'm suggesting a utility that can take a nested object and parse it into a flat object with dot-notation keys. That way you can have a nested object on the client side, parse it into a flat object, send it as multipart/form-data, then use parseBody({ dot: true }) (as implemented here) on the server side to get the original nested object.

Either way, I totally understand if that's out of scope for Hono! Just thought it might be a nice companion to this functionality. It seems like anyone using parseBody({ dot: true }) will end up implementing their own version of what I just described on the client side.