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.

Sign up for GitHub

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 nil exception to empty headers during metadata assignment #140

Merged
merged 4 commits into from
Feb 16, 2023
Merged

Fix nil exception to empty headers during metadata assignment #140

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbbf549
Fix nil exception to empty headers in metadata during event assignment
kaisecheng Feb 15, 2023
9a205f2
changelog
kaisecheng Feb 15, 2023
4c0355b
update test
kaisecheng Feb 15, 2023
c5bddb1
fix test
kaisecheng Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 11.2.1
- Fix nil exception to empty headers of record during event metadata assignment [#140](https://github.com/logstash-plugins/logstash-integration-kafka/pull/140)

## 11.2.0
- Added TLS truststore and keystore settings specifically to access the schema registry [#137](https://github.com/logstash-plugins/logstash-integration-kafka/pull/137)

Expand Down
2 changes: 2 additions & 0 deletions lib/logstash/inputs/kafka.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ def maybe_set_metadata(event, record)
end
if @metadata_mode.include?(:headers)
record.headers.each do |header|
next if header.nil? || header.value.nil? || header.key.nil?
Copy link
Contributor

@andsel andsel Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to comment this line and ran the test, but was still green. Seems the test doesn't enter the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great that you spot it


s = String.from_java_bytes(header.value)
s.force_encoding(Encoding::UTF_8)
if s.valid_encoding?
Expand Down
2 changes: 1 addition & 1 deletion logstash-integration-kafka.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = 'logstash-integration-kafka'
s.version = '11.2.0'
s.version = '11.2.1'
s.licenses = ['Apache-2.0']
s.summary = "Integration with Kafka - input and output plugins"
s.description = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline "+
Expand Down
15 changes: 15 additions & 0 deletions spec/unit/inputs/kafka_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,21 @@
subject.register
expect(subject.metadata_mode).to include(:record_props)
end

it "guards against nil header" do
java_import org.apache.kafka.clients.consumer.ConsumerRecord
java_import org.apache.kafka.common.header.internals.RecordHeaders
java_import org.apache.kafka.common.record.TimestampType

subject.register

evt = LogStash::Event.new('message' => 'Hello')
headers = RecordHeaders.new
record = ConsumerRecord.new("topic", 0, 1234567, 1676475552, TimestampType::CREATE_TIME, 1234567,
0, 0, "k", "v", headers)

expect { subject.maybe_set_metadata(evt, record) }.not_to raise_error
end
end

context 'with client_rack' do
Expand Down