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

BugFix: Make the header if it is empty to fix panic #1724

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

canack
Copy link
Contributor

@canack canack commented Oct 2, 2024

Summary

This pull request fixes a bug in the NATS Go client related to handling message headers in consumers. The issue occurs when setting headers on messages that initially lack them, which causes a panic and crashes the application.

Problem Description

Consumers can successfully set or add headers to received messages if those messages already have headers. However, if a message arrives without headers and the consumer attempts to set a header, it results in a panic.

Solution

The fix ensures that consumers safely check for the presence of headers in a message. If none exist, the header map is initialized to prevent panics, thus enhancing application stability.

Impact

This improvement prevents unexpected crashes, ensuring the NATS Go client handles messages consistently, even those without initial headers.

  Scenario 1: Message with Initial Headers  
  +-----------------+                       +--------------------+  
  |  Producer       |    Send Message       |  Consumer          |  
  |  (with headers) |---------------------->|  Reads Headers     |  
  +-----------------+                       |  Set/Add Header    |  
                                            |  (success)         |  
                                            +--------------------+  

  Scenario 2: Message without Initial Headers  
  +-----------------+                       +--------------------+  
  |  Producer       |    Send Message       |  Consumer          |  
  | (no headers)    |---------------------->|  Attempt Set/Add   |  
  +-----------------+                       |  Header (panic!)   |  
                                            +--------------------+

Example repository I've created to reproduce this scenario: https://github.com/canack/nats-header-problem

@coveralls
Copy link

Coverage Status

coverage: 84.834% (-0.03%) from 84.867%
when pulling f69a1f6 on canack:fix-nil-header-panic
into f0c0194 on nats-io:main.

@canack canack changed the title BugFix: Make the header if it is empty BugFix: Make the header if it is empty to fix panic Oct 2, 2024
@Jarema Jarema requested a review from piotrpio October 16, 2024 11:19
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