martin-g commented on code in PR #3635:
URL: https://github.com/apache/avro/pull/3635#discussion_r2732095067
##########
lang/c/src/avro/io.h:
##########
@@ -52,6 +52,7 @@ avro_reader_memory_set_source(avro_reader_t reader, const
char *buf, int64_t len
void
avro_writer_memory_set_dest(avro_writer_t writer, const char *buf, int64_t
len);
+int64_t avro_max_read(avro_reader_t reader);
Review Comment:
```suggestion
/*
* Returns the number of bytes available to read from the reader.
* For memory readers, returns remaining bytes (len - read).
* For file readers, returns buffered bytes available.
* Returns -1 for unknown reader types.
*/
int64_t avro_max_read(avro_reader_t reader);
```
Is this an accurate documentation ?
##########
lang/c/src/encoding_binary.c:
##########
@@ -136,8 +136,10 @@ static int read_bytes(avro_reader_t reader, char **bytes,
int64_t * len)
avro_set_error("Cannot allocate buffer for bytes value");
return ENOMEM;
}
- AVRO_READ(reader, *bytes, *len);
+
Review Comment:
Shouldn't this also check with `avro_max_read()`, as `read_string()` does
below ?
##########
lang/c/src/encoding_binary.c:
##########
@@ -180,21 +182,30 @@ size_bytes(avro_writer_t writer, const char *bytes, const
int64_t len)
static int read_string(avro_reader_t reader, char **s, int64_t *len)
{
int64_t str_len = 0;
+ int64_t max_available = -1;
int rval;
check_prefix(rval, read_long(reader, &str_len),
"Cannot read string length: ");
if (str_len < 0) {
avro_set_error("Invalid string length: %" PRId64, str_len);
return EINVAL;
}
+ // max := r.tail - r.head + 1; if max >= 0 && size > max
+ max_available = avro_max_read(reader);
+ if (max_available >= 0 && str_len > max_available) {
+ avro_set_error("mem io: String length %" PRId64 " is greater than
available buffer size %" PRId64,
Review Comment:
Please make sure to use the same indentation as the rest of the file, i.e.
tabs.
##########
lang/c/src/encoding_binary.c:
##########
@@ -136,8 +136,10 @@ static int read_bytes(avro_reader_t reader, char **bytes,
int64_t * len)
avro_set_error("Cannot allocate buffer for bytes value");
return ENOMEM;
}
- AVRO_READ(reader, *bytes, *len);
+
(*bytes)[*len] = '\0';
+ AVRO_SAFE_READ(reader, *bytes, *len, *len+1);
Review Comment:
```suggestion
AVRO_SAFE_READ(reader, *bytes, *len, *len+1);
```
##########
lang/c/src/io.c:
##########
@@ -274,6 +274,20 @@ int avro_read(avro_reader_t reader, void *buf, int64_t len)
return EINVAL;
}
+
+int64_t avro_max_read(avro_reader_t reader)
+{
+ if (is_memory_io(reader)) {
+ struct _avro_reader_memory_t *mem_reader =
avro_reader_to_memory(reader);
+ return mem_reader->len - mem_reader->read;
+ } else if (is_file_io(reader)) {
+ struct _avro_reader_file_t *file_reader =
avro_reader_to_file(reader);
+ return bytes_available(file_reader);
Review Comment:
This returns only the buffered bytes, not all remaining bytes as the memory
io branch above
##########
lang/c/src/encoding_binary.c:
##########
@@ -180,21 +182,30 @@ size_bytes(avro_writer_t writer, const char *bytes, const
int64_t len)
static int read_string(avro_reader_t reader, char **s, int64_t *len)
{
int64_t str_len = 0;
+ int64_t max_available = -1;
int rval;
check_prefix(rval, read_long(reader, &str_len),
"Cannot read string length: ");
if (str_len < 0) {
avro_set_error("Invalid string length: %" PRId64, str_len);
return EINVAL;
}
+ // max := r.tail - r.head + 1; if max >= 0 && size > max
+ max_available = avro_max_read(reader);
+ if (max_available >= 0 && str_len > max_available) {
+ avro_set_error("mem io: String length %" PRId64 " is greater than
available buffer size %" PRId64,
Review Comment:
```suggestion
avro_set_error("String length %" PRId64 " is greater than available
buffer size %" PRId64,
```
It is used for both `mem` and `file` reads.
##########
lang/c/src/encoding_binary.c:
##########
@@ -180,21 +182,30 @@ size_bytes(avro_writer_t writer, const char *bytes, const
int64_t len)
static int read_string(avro_reader_t reader, char **s, int64_t *len)
{
int64_t str_len = 0;
+ int64_t max_available = -1;
int rval;
check_prefix(rval, read_long(reader, &str_len),
"Cannot read string length: ");
if (str_len < 0) {
avro_set_error("Invalid string length: %" PRId64, str_len);
return EINVAL;
}
+ // max := r.tail - r.head + 1; if max >= 0 && size > max
Review Comment:
```suggestion
```
This comment is not very helpful, neither for memory nor for file reads.
Feel free to suggest a better one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
