Enforce non-empty metadata keys in forms #9650

zubron · 2025-11-06T22:55:34Z

Change Description

Background

The UI for adding metadata fields allowed empty key-value pairs to be added. This is different from the CLI behaviour which filters them. While we plan to add server-side validation for empty metadata keys in a future update, for now, we want to provide client-side validation to give immediate user feedback and avoid unnecessary API calls that would fail validation.

Bug Fix

Add client-side validation to prevent form submission (commit, merge, import) when metadata keys are empty or whitespace-only.

Fields are marked as invalid when:

User blurs an empty key input
Form is submitted with empty keys

Invalid fields display "Key is required" error message.

Testing Details

Added component tests using React Testing Library to verify validation behaviour.
Also performed manual testing to verify behaviour when interacting with many fields.

Add client-side validation to prevent form submission (commit, merge, import) when metadata keys are empty or whitespace-only. Fields are marked invalid when: - User blurs an empty key input - Form is submitted with empty keys Invalid fields display "Key is required" error message. Added component tests using React Testing Library to verify validation behavior. Closes #5251

zubron · 2025-11-20T15:15:07Z

@nopcoder - I'm tagging you on this to see if this is the right approach for the UI to take with empty metadata keys. I checked the UI of S3. When using the S3 API directly, they don't error on empty metadata keys, but they do show an error on the UI.

nopcoder · 2025-11-21T07:18:39Z

@nopcoder - I'm tagging you on this to see if this is the right approach for the UI to take with empty metadata keys. I checked the UI of S3. When using the S3 API directly, they don't error on empty metadata keys, but they do show an error on the UI.

@zubron if calling s3 with empty metadata key, do not fail as invalid request - is getting the metadata returns the value? if there is no value or a way to set an empty metadata key object s3, we can make the UI's UX friendly and keep our gateway S3 behaviour the same.

nopcoder · 2025-11-23T10:56:16Z

@nopcoder - I'm tagging you on this to see if this is the right approach for the UI to take with empty metadata keys. I checked the UI of S3. When using the S3 API directly, they don't error on empty metadata keys, but they do show an error on the UI.

@zubron if calling s3 with empty metadata key, do not fail as invalid request - is getting the metadata returns the value? if there is no value or a way to set an empty metadata key object s3, we can make the UI's UX friendly and keep our gateway S3 behaviour the same.

As there is no way to get metadata with empty key and the API just ignore them if you set an empty key - I think that the UX can block (perform validation) setting this case.

itaigilo

Thanks for handling this @zubron ,
Looking pretty good.

There are still some changes required, mainly about tidying up the code.

itaigilo · 2025-11-24T11:38:54Z

webui/src/pages/repositories/repository/objects.jsx

        if (committing) return;
-        setShow(false)
+        setShow(false);
+        setMetadataFields([]);


Why setting to []?
It clears the user's work in case of closing and re-opening, no?

Yes, you're right. This does clear any entries when closing and opening again, and this is modifying the existing behaviour. I think I added this because the commit message is not persisted across the same flow so it seemed to stand out that the metadata persisted. Happy to revert it though!

itaigilo · 2025-11-24T11:40:47Z

webui/src/lib/components/repository/compareBranchesActionBar.jsx

    }

    const onSubmit = async () => {
+        if (!validateMetadataKeys(metadataFields, setMetadataFields)) {


This part repeats itself...

How about extracting metadataFields.forEach(pair => metadata[pair.key] = pair.value) into a function, and validateMetadataKeys() in this extracted function?

itaigilo · 2025-11-24T11:41:26Z

webui/test/setup.js

Why is this needed?

This file is needed to add test matchers like toBeInTheDocument() and toHaveValue() to Vitest, and to automatically clean up React components between tests.

itaigilo · 2025-11-24T11:44:25Z

webui/src/lib/components/repository/metadata.jsx

+                                />
+                                {showError && (
+                                    <Form.Control.Feedback type="invalid">
+                                        Key is required


Please make this more generic:
Something like:

const fieldError = getFieldError(f); ... {fieldError && (<Form.Control.Feedback type="invalid">{fieldError}</Form.Control>)}

itaigilo · 2025-11-24T11:50:31Z

webui/src/lib/components/repository/metadata.jsx

+ * @param {Function} setMetadataFields - Setter function to update metadata fields
+ * @returns {boolean} True if validation passed (no empty keys), false if validation failed
+ */
+export const validateMetadataKeys = (metadataFields, setMetadataFields) => {


Seems like mixing two things here:
I don't expect a validation function to also update the state (in this case, by using setMetadataFields).

I think it's better to split it into hasEmptyKeys() and touchEmptyKeys(),
But maybe there's a nicer solution.

Yeah, you're right, that shouldn't be happening in this function. I've separated this into two pure functions which returns responsibility for updating state back to the parent component, and should also help with the duplication that you mentioned in another comment above.

zubron · 2025-11-24T13:09:20Z

As there is no way to get metadata with empty key and the API just ignore them if you set an empty key - I think that the UX can block (perform validation) setting this case.

Thanks for taking a look at this, @nopcoder. Yes, my understanding is that there is no way to retrieve the metadata with the empty key. When a user tries to retrieve the object, the response will include the x-amz-missing-meta header with how many entries can't be returned via HTTP headers. I believe this header was originally used for the SOAP API but that has since been deprecated (EOL October 2025), and is not recommended for use.

https://gist.github.com/zubron/783dc8ccdaba2108a5453086031e8785#2-empty-keys

nopcoder · 2025-11-24T13:20:55Z

As there is no way to get metadata with empty key and the API just ignore them if you set an empty key - I think that the UX can block (perform validation) setting this case.

Thanks for taking a look at this, @nopcoder. Yes, my understanding is that there is no way to retrieve the metadata with the empty key. When a user tries to retrieve the object, the response will include the x-amz-missing-meta header with how many entries can't be returned via HTTP headers. I believe this header was originally used for the SOAP API but that has since been deprecated (EOL October 2025), and is not recommended for use.

https://gist.github.com/zubron/783dc8ccdaba2108a5453086031e8785#2-empty-keys

#zubron, So the only issue we still need to fix is the backend storing an empty key's value, right? (not as part of this PR)

zubron · 2025-11-24T13:48:14Z

#zubron, So the only issue we still need to fix is the backend storing an empty key's value, right? (not as part of this PR)

Yep, I'll open another issue for that 👍

Validation functions do not modify state, instead makes it the parent component's responsibility.

itaigilo

LGTM 😄

zubron mentioned this pull request Nov 6, 2025

UI: On commit - empty metadata keys are allowed #5251

Open

zubron force-pushed the fix/empty-metadata-keys-are-allowed-5251 branch 2 times, most recently from 8047280 to 5acff90 Compare November 17, 2025 21:19

zubron added area/UI Improvements or additions to UI include-changelog PR description should be included in next release changelog labels Nov 17, 2025

zubron changed the title ~~Filter empty key-value metadata pairs in UI~~ Prevent submission of empty metadata keys Nov 17, 2025

zubron force-pushed the fix/empty-metadata-keys-are-allowed-5251 branch 4 times, most recently from 33df7ec to 19508be Compare November 18, 2025 19:20

zubron changed the title ~~Prevent submission of empty metadata keys~~ Enforce non-empty metadata keys in forms Nov 18, 2025

zubron marked this pull request as ready for review November 18, 2025 19:31

zubron force-pushed the fix/empty-metadata-keys-are-allowed-5251 branch from 19508be to 47c9992 Compare November 18, 2025 19:34

zubron requested review from itaigilo and nopcoder November 20, 2025 14:52

itaigilo requested changes Nov 24, 2025

View reviewed changes

Address review comments

81ceb24

Validation functions do not modify state, instead makes it the parent component's responsibility.

zubron force-pushed the fix/empty-metadata-keys-are-allowed-5251 branch from 01834e0 to 81ceb24 Compare November 26, 2025 21:23

zubron requested a review from itaigilo November 26, 2025 21:23

zubron mentioned this pull request Nov 26, 2025

Drop metadata with empty keys #9725

Open

itaigilo approved these changes Nov 26, 2025

View reviewed changes

Enforce non-empty metadata keys in forms #9650

Are you sure you want to change the base?

Enforce non-empty metadata keys in forms #9650

Uh oh!

Conversation

zubron commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Description

Background

Bug Fix

Testing Details

Uh oh!

zubron commented Nov 20, 2025

Uh oh!

nopcoder commented Nov 21, 2025

Uh oh!

nopcoder commented Nov 23, 2025

Uh oh!

itaigilo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zubron commented Nov 24, 2025

Uh oh!

nopcoder commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zubron commented Nov 24, 2025

Uh oh!

itaigilo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zubron commented Nov 6, 2025 •

edited

Loading

nopcoder commented Nov 24, 2025 •

edited

Loading