iemejia commented on code in PR #3857:
URL: https://github.com/apache/avro/pull/3857#discussion_r3567291867
##########
lang/csharp/src/apache/main/File/Codec.cs:
##########
@@ -29,6 +29,87 @@ namespace Avro.File
/// </summary>
public abstract class Codec
{
+ /// <summary>
+ /// Default upper bound, in bytes, on the size a single data-file
block may
+ /// decompress to. A block with a very high compression ratio (or a
malformed
+ /// block) can otherwise expand to far more memory than its compressed
size.
+ /// Mirrors the Java SDK's decompression limit (AVRO-4247).
Overridable with
+ /// the AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+ /// </summary>
+ public const long DefaultMaxDecompressLength = 200L * 1024 * 1024; //
200 MiB
Review Comment:
Good point — changed `DefaultMaxDecompressLength` to `static readonly` in
51bf9f85c0 so the value isn't inlined into referencing assemblies and a future
default change won't require recompiling them.
##########
lang/csharp/src/apache/main/File/Codec.cs:
##########
@@ -29,6 +29,87 @@ namespace Avro.File
/// </summary>
public abstract class Codec
{
+ /// <summary>
+ /// Default upper bound, in bytes, on the size a single data-file
block may
+ /// decompress to. A block with a very high compression ratio (or a
malformed
+ /// block) can otherwise expand to far more memory than its compressed
size.
+ /// Mirrors the Java SDK's decompression limit (AVRO-4247).
Overridable with
+ /// the AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+ /// </summary>
+ public const long DefaultMaxDecompressLength = 200L * 1024 * 1024; //
200 MiB
+
+ /// <summary>
+ /// Name of the environment variable used to override the default
maximum
+ /// decompressed size of a single block.
+ /// </summary>
+ public const string MaxDecompressLengthEnvVar =
"AVRO_MAX_DECOMPRESS_LENGTH";
Review Comment:
Agreed — left `MaxDecompressLengthEnvVar` as `const`; it's a stable string
identifier that referencing assemblies inlining is fine for.
##########
lang/csharp/src/apache/main/File/Codec.cs:
##########
@@ -29,6 +29,87 @@ namespace Avro.File
/// </summary>
public abstract class Codec
{
+ /// <summary>
+ /// Default upper bound, in bytes, on the size a single data-file
block may
+ /// decompress to. A block with a very high compression ratio (or a
malformed
+ /// block) can otherwise expand to far more memory than its compressed
size.
+ /// Mirrors the Java SDK's decompression limit (AVRO-4247).
Overridable with
+ /// the AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+ /// </summary>
+ public const long DefaultMaxDecompressLength = 200L * 1024 * 1024; //
200 MiB
+
+ /// <summary>
+ /// Name of the environment variable used to override the default
maximum
+ /// decompressed size of a single block.
+ /// </summary>
+ public const string MaxDecompressLengthEnvVar =
"AVRO_MAX_DECOMPRESS_LENGTH";
+
+ /// <summary>
+ /// The maximum number of bytes a single block is allowed to
decompress to.
+ /// </summary>
+ /// <returns>The configured limit, honoring the environment
override.</returns>
+ public static long GetMaxDecompressLength()
+ {
+ var value =
Environment.GetEnvironmentVariable(MaxDecompressLengthEnvVar);
+ if (value != null && long.TryParse(value, out var parsed) &&
parsed > 0)
Review Comment:
Done in 51bf9f85c0 — the override is now parsed with `NumberStyles.Integer,
CultureInfo.InvariantCulture` so it's culture-independent.
##########
lang/csharp/src/apache/main/File/Codec.cs:
##########
@@ -29,6 +29,87 @@ namespace Avro.File
/// </summary>
public abstract class Codec
{
+ /// <summary>
+ /// Default upper bound, in bytes, on the size a single data-file
block may
+ /// decompress to. A block with a very high compression ratio (or a
malformed
+ /// block) can otherwise expand to far more memory than its compressed
size.
+ /// Mirrors the Java SDK's decompression limit (AVRO-4247).
Overridable with
+ /// the AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+ /// </summary>
+ public const long DefaultMaxDecompressLength = 200L * 1024 * 1024; //
200 MiB
+
+ /// <summary>
+ /// Name of the environment variable used to override the default
maximum
+ /// decompressed size of a single block.
+ /// </summary>
+ public const string MaxDecompressLengthEnvVar =
"AVRO_MAX_DECOMPRESS_LENGTH";
+
+ /// <summary>
+ /// The maximum number of bytes a single block is allowed to
decompress to.
+ /// </summary>
+ /// <returns>The configured limit, honoring the environment
override.</returns>
+ public static long GetMaxDecompressLength()
+ {
+ var value =
Environment.GetEnvironmentVariable(MaxDecompressLengthEnvVar);
+ if (value != null && long.TryParse(value, out var parsed) &&
parsed > 0)
+ {
+ return parsed;
+ }
+
+ return DefaultMaxDecompressLength;
+ }
+
+ /// <summary>
+ /// Throws if the given decompressed length exceeds the maximum
allowed.
+ /// </summary>
+ /// <param name="length">The number of decompressed bytes.</param>
+ /// <param name="maxLength">The maximum number of decompressed bytes
allowed.</param>
+ public static void CheckDecompressLength(long length, long maxLength)
+ {
+ if (length > maxLength)
+ {
+ throw new AvroRuntimeException(
+ $"Decompressed block size {length} exceeds the maximum
allowed of {maxLength} bytes. " +
+ $"Set the {MaxDecompressLengthEnvVar} environment variable
to raise the limit.");
Review Comment:
Right. When called from `CopyBounded` the streaming codecs (deflate/gzip)
can't know the total uncompressed length up front —
`DeflateStream.Length`/`GZipStream.Length` throw `NotSupportedException`, and
reading to the end to measure it would itself allow the DoS we're preventing.
So the incremental cap (rejecting as soon as the running total exceeds the
limit) is the best available behavior; the message reports the limit and the
env var to raise it. Left as-is.
--
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]
