Skip to content

fix: crash when tags value are nullptr#7

Merged
dmehala merged 1 commit intomainfrom
dmehala/fix/crash-during-span-construction
Sep 16, 2024
Merged

fix: crash when tags value are nullptr#7
dmehala merged 1 commit intomainfrom
dmehala/fix/crash-during-span-construction

Conversation

@dmehala
Copy link
Copy Markdown
Contributor

@dmehala dmehala commented Sep 16, 2024

Description

Fix issue where empty tag values cause an unhandled exception during string construction.

This commit ensures that string building is prevented if a C-string pointer is empty, avoiding potential crashes.

Fix issue where empty tag values cause an unhandled exception
during string construction.

This commit ensures that string building is prevented if a C-string
pointer is empty, avoiding potential crashes.
@dmehala dmehala requested a review from a team as a code owner September 16, 2024 10:23
@dmehala dmehala requested review from cataphract and pablomartinezbernardo and removed request for a team September 16, 2024 10:23
@dubloom
Copy link
Copy Markdown
Contributor

dubloom commented Sep 16, 2024

If you look at the docs :

  • using map->merge(source), if a key is already present in map, it won't be extracted from source. Therefore the base code overrides the value from the initial map
  • using map->emplace, if the element is already in the map, it is not inserted. Therefore, if the tags map already has one of the key, it will be keep the value it had.

Is this change of behavior desired ?

@dmehala
Copy link
Copy Markdown
Contributor Author

dmehala commented Sep 16, 2024

@dubloom Yes.

@dmehala dmehala merged commit 05706df into main Sep 16, 2024
@dmehala dmehala deleted the dmehala/fix/crash-during-span-construction branch September 16, 2024 14:42
pawelchcki pushed a commit that referenced this pull request Feb 2, 2026
Co-authored-by: campaigner-prod[bot] <87874424+campaigner-prod[bot]@users.noreply.github.com>
pawelchcki pushed a commit that referenced this pull request Feb 2, 2026
Since #256 the CI started to crash for ingress-nginx 1.13.3 on amd64. I
traced the issue back to `ngx_set_env` which crashes because of the bump
to OpenResty. More details in #7.
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.

3 participants