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

the WriteApi does not check whether a tag value contains unauthorized characters ("\n","\r"...) #127

Closed
AnesBendimerad opened this issue Jun 24, 2020 · 7 comments · Fixed by #129
Assignees
Labels
bug Something isn't working
Milestone

Comments

@AnesBendimerad
Copy link

InfluxDB has some restrictions on tag values : they cannot contain some characters such as: line feed (\n), carriage return (\r). Currently, the Java client library does not check whether values of tags are safe (free of unauthorized characters) before writing them into InfluxDB.
For example, I have a measurement class called Temperature, and it contains a tag field location. When I put location="mad\nrid" and I write it using WriteApi, it will throw the following exception: partial write: unable to parse 'temperature,location=mad': missing fields dropped=0. So a partial write will be done in the influxDB database: a new measurement rid will be mistakenly created, and it will contain only a part of the write.
If a tag value contains '\r', the java client will not even throw an exception, and it will perform an incorrect write into the database.

I think it would be really useful to add a method that ensures that tag values are correct before sending them to InfluxDB through the line protocol, in order to avoid incorrect writes and anomalous behaviors.

@mehdi-kaytoue
Copy link

mehdi-kaytoue commented Jun 24, 2020

I guess the same problem occurs in the previous Java library influxdb-java
The most serious problem was, as you mention, the creation of wrong measurements in presence of '\r'.

@bednar
Copy link
Contributor

bednar commented Jun 24, 2020

Thanks for reporting the issue.

I think that the problem is in the escaping of tag values. If the character is \n then character should be escaped into \\n. The data stored in InfluxDB will be looks like:

Screenshot 2020-06-24 at 16 00 19

What do you think?

@bednar bednar added bug Something isn't working state: in progress labels Jun 24, 2020
@bednar bednar self-assigned this Jun 24, 2020
@patrick-schweizer
Copy link

a similar problem occurs for :and =

@bednar
Copy link
Contributor

bednar commented Jun 25, 2020

@patrick-schweizer Could you please add a detail information about problem with : and =? I tried it by the following snipped and these characters are serialized correctly:

Point point = Point.measurement("characters")
                .addTag("hu:ps", "test")
                .addTag("hu=ps", "test")
                .addField("value", 28.0);

System.out.println("LP: " + point.toLineProtocol());
LP: characters,hu:ps=test,hu\=ps=test value=28.0

@patrick-schweizer
Copy link

patrick-schweizer commented Jun 25, 2020

@patrick-schweizer Could you please add a detail information about problem with : and =? I tried it by the following snipped and these characters are serialized correctly:

Point point = Point.measurement("characters")
                .addTag("hu:ps", "test")
                .addTag("hu=ps", "test")
                .addField("value", 28.0);

System.out.println("LP: " + point.toLineProtocol());
LP: characters,hu:ps=test,hu\=ps=test value=28.0

@bednar thanks for testing. Sorry I was not very clear in my description. The problem is when having an "=" in the measurement column. I wrote a test that shows the problem:

    public void shouldWorkForMeasurementsWithEquals() throws InterruptedException {
        DateTimeFormatter format = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'").withZone(ZoneId.of("UTC"));

        // write measurement with '='
        Point point = Point
                .measurement("measurement=1")
                .addField("value", 1)
                .time(Instant.now(), WritePrecision.MS);
        influxDBClient.getWriteApi().writePoint(configBucket, configOrg, point);
        influxDBClient.getWriteApi().flush();
        Thread.sleep(2000); // wait for a bit to make sure data was saved.

        // load measurements
        String query = "from(bucket: \"opennms\")" +
                "  |> range(start: -1h, stop: now())" +
                "  |> distinct(column: \"_measurement\")" +
                "  |> sort()";
        List<String> measurements = this.influxDBClient.getQueryApi()
                .query(query)
                .stream()
                .map(FluxTable::getRecords)
                .flatMap(Collection::stream)
                .map(FluxRecord::getValues)
                .map(m -> m.get("_measurement"))
                .filter(Objects::nonNull)
                .map(Object::toString)
                .distinct()
                .collect(Collectors.toList());

        // validate measurement
        assertEquals(1, measurements.size()); // we expect one measurement
        assertTrue(measurements.contains("measurement=1")); // <-- FAIL, it is "measurement\=1" (decoding ist missing)

        // load data
        query = "from(bucket:\"" + this.configBucket + "\")\n" +
                " |> range(start:" + format.format(Instant.now().minus(5, ChronoUnit.MINUTES)) + ", stop:" + format.format(Instant.now()) + ")\n" +
                " |> filter(fn:(r) => r._measurement == \"measurement=1\")\n" +
                " |> filter(fn: (r) => r._field == \"value\")";
        List<FluxTable> tables = this.influxDBClient.getQueryApi().query(query);
        assertEquals(1, tables.size()); // <-- FAIL, we expect one table but got none

        // try to load data again, this time let's escape measurement
        query = "from(bucket:\"" + this.configBucket + "\")\n" +
                " |> range(start:" + format.format(Instant.now().minus(5, ChronoUnit.MINUTES)) + ", stop:" + format.format(Instant.now()) + ")\n" +
                " |> filter(fn:(r) => r._measurement == \"measurement\\=1\")\n" + // name is escaped
                " |> filter(fn: (r) => r._field == \"value\")";
        tables = this.influxDBClient.getQueryApi().query(query); // <-- Exception: com.influxdb.exceptions.InternalServerErrorException: compilation failed: loc 3:5-4:43: expected RPAREN, got EOF
        assertEquals(1, tables.size());
    }

@bednar
Copy link
Contributor

bednar commented Jun 26, 2020

@patrick-schweizer thanks, The = is valid character for measurement name. I will improve PR #129.

@patrick-schweizer
Copy link

@patrick-schweizer thanks, The = is valid character for measurement name. I will improve PR #129.

perfect, thank you! :-)

@bednar bednar added this to the 1.10.0 milestone Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants