Skip to content
This repository was archived by the owner on May 7, 2024. It is now read-only.

Conversation

@marckhouzam
Copy link
Contributor

Opening a new PR with a better from branch.

When updating a plugin configuration, if the element is an array
and the update is to remove all elements from that list,
Konga was sending an empty string to represent the empty list.

When using the CORS plugin, the plugin code checks for the list to be
null, but not always for an empty list.

This patch makes Konga send a null instead of an empty string
when updating a plugin list to become an empty list.

Note: we are currently running Konga 0.13.0 because we have yet to migrate to Kong 1.0 (we have plugins that need to be updated first). That is why I made this change on the legacy branch. If you think this change is safe and you want it on master, just let me know and I can make another PR.

When updating a plugin configuration, if the element is an array
and the update is to remove all elements from that list,
Konga was sending an empty string to represent the empty list.

When using the CORS plugin, the plugin code checks for the list to be
null, but not always for an empty list.

This patch makes Konga send a null instead of an empty string
when updating a plugin list to an empty list.

Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
@pantsel pantsel merged commit 9df2f88 into pantsel:legacy Oct 11, 2019
@marckhouzam marckhouzam deleted the fix/useNullForEmptyLists branch October 11, 2019 12:04
@jeremyjpj0916
Copy link
Contributor

I just hit this case testing in konga, yeah when list is empty in UI we want it to set as nil so Kong can default things like METHODS for instance.

@marckhouzam
Copy link
Contributor Author

I never got around to make a similar PR on the master branch, if it needs it.

Is someone else has the time, don't hesitate.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants