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

Fix error when loading a config with only some services using OIDC #893

Merged
merged 3 commits into from
Sep 13, 2018

Conversation

davidor
Copy link
Contributor

@davidor davidor commented Sep 13, 2018

APIcast crashes when loading a config where only some of the services use OIDC.

The reason is that we create an array of OIDC configs that has size=number_of_services. Depending on the number of services and the number of services that use OIDC, we might create a sparse array. cjson raises an error when trying to convert a sparse array. Check
https://www.kyne.com.au/~mark/software/lua-cjson-manual.html#encode_sparse_array
for more details.

This PR solves the issue by assigning elements of that array to false instead of nil to avoid creating sparse arrays.

Ref: https://issues.jboss.org/browse/THREESCALE-1289

@davidor davidor requested a review from a team as a code owner September 13, 2018 09:19
APIcast crashes when loading a config where only some of the services use
OIDC.

The reason is that we created an array of OIDC configs with
size=number_of_services. Let's say we have 100 services and only the
50th has an OIDC config. In this case, we created this Lua table: { [50]
= oidc_config_here }.

The problem is that cjson raises an error when trying to convert a
sparse array like that into JSON.

Check
https://www.kyne.com.au/~mark/software/lua-cjson-manual.html#encode_sparse_array
for more details.

Assigning false instead of nil solves the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants