Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Do not add null or whitespace lines to payload in ElasticsearchPayloa… #415

Conversation

andrey-kozlov-skuvault
Copy link
Contributor

What issue does this PR address?
This is done to avoid NullReferenceException in Elasticsearch.Net.PostData.Write method in line 204 when we getting bytes from possibly null object var bytes = enumerator.Current.Utf8Bytes();

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

Other information:
These tests fail before that change:

using System;
using System.IO;
using System.Text;
using Elasticsearch.Net;
using FluentAssertions;
using Serilog.Sinks.Elasticsearch.Durable;
using Xunit;

namespace Serilog.Sinks.Elasticsearch.Tests;

public class ElasticsearchPayloadReaderTests : IDisposable
{
    private readonly string _tempFileFullPath;

    public ElasticsearchPayloadReaderTests()
    {
        _tempFileFullPath = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}-{20000101}.json");
    }

    public void Dispose()
    {
        System.IO.File.Delete(_tempFileFullPath);
    }

    [Theory]
    [InlineData("")]
    [InlineData(null)]
    [InlineData(" ")]
    public void ReadPayload_SkipsEmptyLines(string emptyLine)
    {
        // Arrange
        var payloadReader = new ElasticsearchPayloadReader("testPipelineName", "TestTypeName", null,
            (_, _) => "TestIndex", ElasticOpType.Index);
        var lines = new[]
        {
            "line1",
            emptyLine,
            "line2"
        };
        // Important to use UTF8 with BOM if we are starting from 0 position 
        System.IO.File.WriteAllLines(_tempFileFullPath, lines, new UTF8Encoding(true));

        // Act
        var fileSetPosition = new FileSetPosition(0, _tempFileFullPath);
        var count = 0;
        var payload = payloadReader.ReadPayload(int.MaxValue, null, ref fileSetPosition, ref count, _tempFileFullPath);

        // Assert
        // Before the fix, these lines would provide NullReference exception
        var data = PostData.MultiJson(payload);
        data.Write(new MemoryStream(), new ConnectionConfiguration());
    }
}

…dReader

This is done to avoid NullReferenceException in `Elasticsearch.Net.PostData.Write` method in line 204 when we getting bytes from possibly null object `var bytes = enumerator.Current.Utf8Bytes();`
…load_reader

# Conflicts:
#	test/Serilog.Sinks.Elasticsearch.Tests/ElasticsearchPayloadReaderTests.cs
@mivano mivano closed this Jun 1, 2024
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.

None yet

2 participants