Skip to content

Commit f793274

Browse files
mikemcdougallMike McDougall
mikemcdougall
and
Mike McDougall
authored
fix: address review findings (#211) (#215)
* feat: add load/soak tests and nightly job (#202) * test: reduce streaming perf test flakiness (#202) * fix: address review findings (#211) * fix: implement limited stream flush * fix: use memory-based read async * test: fix analyzer warnings * test: stabilize memory pooling perf test * test: stabilize coordinate pooling perf test * Fix architecture gate violations --------- Co-authored-by: Mike McDougall <mike@honua.io>
1 parent d120d32 commit f793274

File tree

13 files changed

+544
-100
lines changed

13 files changed

+544
-100
lines changed

‎src/Honua.Postgres/Features/Import/InMemoryImportJobService.cs‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,29 @@ public async Task<string> QueueImportAsync(
4040
long fileSize,
4141
CancellationToken cancellationToken = default)
4242
{
43-
var jobId = Guid.NewGuid().ToString("N")[..8];
4443
var format = _importService.DetectFormat(request.FileName) ?? SupportedFileFormat.GeoJson;
4544
var formatName = format.ToString();
4645

47-
var progress = ImportProgress.CreateInitial(jobId, request.TableName, format, fileSize);
48-
var state = new ImportJobState
46+
ImportJobState state;
47+
string jobId;
48+
while (true)
4949
{
50-
Progress = progress,
51-
Request = request,
52-
StartedAt = DateTimeOffset.UtcNow,
53-
FileSize = fileSize,
54-
Format = formatName
55-
};
56-
57-
_jobs[jobId] = state;
50+
jobId = Guid.NewGuid().ToString("N")[..8];
51+
var progress = ImportProgress.CreateInitial(jobId, request.TableName, format, fileSize);
52+
state = new ImportJobState
53+
{
54+
Progress = progress,
55+
Request = request,
56+
StartedAt = DateTimeOffset.UtcNow,
57+
FileSize = fileSize,
58+
Format = formatName
59+
};
60+
61+
if (_jobs.TryAdd(jobId, state))
62+
{
63+
break;
64+
}
65+
}
5866

5967
var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
6068
_cancellationTokens[jobId] = cts;

‎src/Honua.Server/Features/FeatureServer/AttachmentEndpoints.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Honua.Core.Features.Attachments.Abstractions;
66
using Honua.Core.Features.Catalog.Abstractions;
77
using Honua.Server.Features.Infrastructure.Helpers;
8+
using Honua.Server.Features.Infrastructure.Security;
89
using Microsoft.Extensions.Options;
910

1011
namespace Honua.Server.Features.FeatureServer;
@@ -135,6 +136,7 @@ private static async Task HandleAddAttachment(HttpContext context)
135136

136137
var attachmentStore = context.RequestServices.GetRequiredService<IAttachmentStore>();
137138
var limitsOptions = context.RequestServices.GetRequiredService<IOptions<LimitsOptions>>();
139+
var securityOptions = context.RequestServices.GetRequiredService<IOptions<FileUploadSecurityOptions>>();
138140
var logger = context.RequestServices.GetRequiredService<ILogger<AttachmentOperations>>();
139141

140142
var result = await AddAttachmentAsync(
@@ -144,6 +146,7 @@ private static async Task HandleAddAttachment(HttpContext context)
144146
keywords,
145147
attachmentStore,
146148
limitsOptions.Value.Attachments,
149+
securityOptions.Value,
147150
logger,
148151
context.RequestAborted);
149152

@@ -320,8 +323,17 @@ private static async Task HandleDownloadAttachment(HttpContext context)
320323
private static Task<IResult> QueryAttachmentsAsync(int layerId, long featureId, IAttachmentStore attachmentStore, ILogger<AttachmentOperations> logger, CancellationToken cancellationToken)
321324
=> AttachmentHandler.QueryAttachmentsAsync(layerId, featureId, attachmentStore, logger, cancellationToken);
322325

323-
private static Task<IResult> AddAttachmentAsync(int layerId, long featureId, IFormFile file, string? keywords, IAttachmentStore attachmentStore, AttachmentLimits limits, ILogger<AttachmentOperations> logger, CancellationToken cancellationToken)
324-
=> AttachmentHandler.AddAttachmentAsync(layerId, featureId, file, keywords, attachmentStore, limits, logger, cancellationToken);
326+
private static Task<IResult> AddAttachmentAsync(
327+
int layerId,
328+
long featureId,
329+
IFormFile file,
330+
string? keywords,
331+
IAttachmentStore attachmentStore,
332+
AttachmentLimits limits,
333+
FileUploadSecurityOptions securityOptions,
334+
ILogger<AttachmentOperations> logger,
335+
CancellationToken cancellationToken)
336+
=> AttachmentHandler.AddAttachmentAsync(layerId, featureId, file, keywords, attachmentStore, limits, securityOptions, logger, cancellationToken);
325337

326338
private static Task<IResult> UpdateAttachmentAsync(int layerId, long featureId, long attachmentId, string? keywords, IAttachmentStore attachmentStore, ILogger<AttachmentOperations> logger, CancellationToken cancellationToken)
327339
=> AttachmentHandler.UpdateAttachmentAsync(layerId, featureId, attachmentId, keywords, attachmentStore, logger, cancellationToken);

‎src/Honua.Server/Features/FeatureServer/AttachmentHandler.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public static async Task<IResult> AddAttachmentAsync(
6666
string? keywords,
6767
IAttachmentStore attachmentStore,
6868
AttachmentLimits limits,
69+
FileUploadSecurityOptions securityOptions,
6970
ILogger<AttachmentOperations> logger,
7071
CancellationToken cancellationToken)
7172
{
@@ -99,7 +100,10 @@ public static async Task<IResult> AddAttachmentAsync(
99100
}
100101

101102
// Security Layer 4: Validate file content for malicious signatures
102-
var contentValidation = await FileUploadSecurity.ValidateFileContentAsync(file, cancellationToken);
103+
var contentValidation = await FileUploadSecurity.ValidateFileContentAsync(
104+
file,
105+
securityOptions.MaxSecurityScanSizeBytes,
106+
cancellationToken);
103107
if (!contentValidation.IsValid)
104108
{
105109
LogSecurityValidationFailed(logger, layerId, featureId, "content", contentValidation.ErrorMessage);

‎src/Honua.Server/Features/FileStorage/LocalFileStorage.cs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internal sealed class LocalFileStorage : ICloudFileStorage
1919
private readonly LocalStorageOptions _options;
2020
private readonly ILogger<LocalFileStorage> _logger;
2121
private readonly ConcurrentDictionary<string, CloudFile> _fileIndex;
22-
private readonly ConcurrentDictionary<string, HashSet<string>> _batchIndex;
22+
private readonly ConcurrentDictionary<string, ConcurrentDictionary<string, byte>> _batchIndex;
2323
private readonly string _basePath;
2424
private readonly string _metadataPath;
2525
private static readonly JsonSerializerOptions _jsonOptions = new()
@@ -39,10 +39,12 @@ public LocalFileStorage(
3939
{
4040
_options = options.Value ?? throw new ArgumentNullException(nameof(options));
4141
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
42-
_basePath = _options.BasePath;
42+
_basePath = string.IsNullOrWhiteSpace(_options.BasePath)
43+
? Directory.GetCurrentDirectory()
44+
: Path.TrimEndingDirectorySeparator(_options.BasePath);
4345
_metadataPath = Path.Combine(_basePath, ".metadata");
4446
_fileIndex = new ConcurrentDictionary<string, CloudFile>();
45-
_batchIndex = new ConcurrentDictionary<string, HashSet<string>>();
47+
_batchIndex = new ConcurrentDictionary<string, ConcurrentDictionary<string, byte>>();
4648

4749
InitializeStorage();
4850
}
@@ -206,7 +208,7 @@ public async Task<bool> DeleteAsync(string fileId, CancellationToken cancellatio
206208
// Remove from batch index if present
207209
foreach (var batch in _batchIndex)
208210
{
209-
batch.Value.Remove(fileId);
211+
_ = batch.Value.TryRemove(fileId, out _);
210212
}
211213

212214
FileStorageLog.FileDeleted(_logger, fileId);
@@ -231,7 +233,7 @@ public async Task<BatchUploadResult> UploadBatchAsync(BatchUploadRequest request
231233
var uploadedFiles = new List<CloudFile>();
232234
var failedFiles = new Dictionary<string, string>();
233235

234-
_batchIndex[batchId] = new HashSet<string>();
236+
_batchIndex[batchId] = new ConcurrentDictionary<string, byte>();
235237

236238
foreach (var file in request.Files)
237239
{
@@ -252,7 +254,7 @@ public async Task<BatchUploadResult> UploadBatchAsync(BatchUploadRequest request
252254
if (result.Success && result.File is not null)
253255
{
254256
uploadedFiles.Add(result.File);
255-
_batchIndex[batchId].Add(result.File.FileId);
257+
_ = _batchIndex[batchId].TryAdd(result.File.FileId, 0);
256258
}
257259
else
258260
{
@@ -293,7 +295,7 @@ public async Task<int> DeleteBatchAsync(string batchId, CancellationToken cancel
293295
}
294296

295297
var deletedCount = 0;
296-
foreach (var fileId in fileIds)
298+
foreach (var fileId in fileIds.Keys)
297299
{
298300
if (await DeleteAsync(fileId, cancellationToken))
299301
{
@@ -448,8 +450,8 @@ private void LoadExistingMetadata()
448450
// Rebuild batch index
449451
if (cloudFile.Metadata.TryGetValue("BatchId", out var batchId))
450452
{
451-
var batchFiles = _batchIndex.GetOrAdd(batchId, _ => new HashSet<string>());
452-
batchFiles.Add(cloudFile.FileId);
453+
var batchFiles = _batchIndex.GetOrAdd(batchId, _ => new ConcurrentDictionary<string, byte>());
454+
_ = batchFiles.TryAdd(cloudFile.FileId, 0);
453455
}
454456
}
455457
}
@@ -475,13 +477,38 @@ private string GetMetadataFilePath(string fileId) =>
475477
private static string GenerateFileId() =>
476478
Guid.NewGuid().ToString("N");
477479

478-
private static string BuildStoragePath(string fileId, string fileName, string? folder)
480+
private string BuildStoragePath(string fileId, string fileName, string? folder)
479481
{
480482
var extension = Path.GetExtension(fileName);
481483
var storageName = $"{fileId}{extension}";
482484

483-
return string.IsNullOrEmpty(folder)
484-
? storageName
485-
: Path.Combine(folder, storageName);
485+
if (string.IsNullOrWhiteSpace(folder))
486+
{
487+
return storageName;
488+
}
489+
490+
ValidateFolderPath(folder);
491+
492+
return Path.Combine(folder, storageName);
493+
}
494+
495+
private static void ValidateFolderPath(string folder)
496+
{
497+
if (Path.IsPathRooted(folder))
498+
{
499+
throw new ArgumentException("Folder must be a relative path.", nameof(folder));
500+
}
501+
502+
if (folder.IndexOfAny(Path.GetInvalidPathChars()) >= 0)
503+
{
504+
throw new ArgumentException("Folder contains invalid path characters.", nameof(folder));
505+
}
506+
507+
var separators = new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar };
508+
var segments = folder.Split(separators, StringSplitOptions.RemoveEmptyEntries);
509+
if (segments.Any(segment => segment == ".."))
510+
{
511+
throw new ArgumentException("Folder must not contain relative traversal segments.", nameof(folder));
512+
}
486513
}
487514
}

‎src/Honua.Server/Features/Import/ImportEndpoints.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Honua.Server.Features.Infrastructure.Authentication;
77
using Honua.Server.Features.Infrastructure.Models;
88
using Honua.Server.Features.Infrastructure.Security;
9+
using Microsoft.Extensions.Options;
910

1011
namespace Honua.Server.Features.Import;
1112

@@ -158,9 +159,11 @@ await ProblemDetailsHelpers.CreateAdminProblem(
158159
}
159160

160161
IFileImportService importService = context.RequestServices.GetRequiredService<IFileImportService>();
162+
var securityOptions = context.RequestServices.GetRequiredService<IOptions<FileUploadSecurityOptions>>();
161163
var previewValidation = await FileUploadSecurity.ValidateFileAsync(
162164
file,
163165
importService.Limits.MaxPreviewSizeBytes,
166+
securityOptions.Value.MaxSecurityScanSizeBytes,
164167
cancellationToken);
165168
if (!previewValidation.IsValid)
166169
{
@@ -244,10 +247,12 @@ await WriteErrorAsync(context, "Invalid table name. Use only letters, numbers, a
244247
}
245248

246249
IFileImportService importService = context.RequestServices.GetRequiredService<IFileImportService>();
250+
var securityOptions = context.RequestServices.GetRequiredService<IOptions<FileUploadSecurityOptions>>();
247251
var maxFileSizeBytes = Math.Max(importService.Limits.BackgroundJobThresholdBytes, importService.Limits.MaxMemoryBytes);
248252
var uploadValidation = await FileUploadSecurity.ValidateFileAsync(
249253
file,
250254
maxFileSizeBytes,
255+
securityOptions.Value.MaxSecurityScanSizeBytes,
251256
cancellationToken);
252257
if (!uploadValidation.IsValid)
253258
{

‎src/Honua.Server/Features/Infrastructure/Caching/ETagService.cs‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ internal sealed class ETagService : IETagService
1616
{
1717
private const int HashSize = 32; // SHA256 hash size in bytes
1818
private const int Base64HashSize = 44; // Base64-encoded SHA256 size (rounded up to nearest 4-byte boundary)
19+
// Base64-encoded SHA256 hash of empty content with padding trimmed to match ComputeETag formatting.
20+
private const string EmptyContentHash = "47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU";
1921

2022
/// <summary>
2123
/// Computes an ETag for a given object by serializing it and hashing the content.
@@ -44,8 +46,8 @@ public string ComputeETag(ReadOnlySpan<byte> data)
4446
{
4547
if (data.IsEmpty)
4648
{
47-
// Return a consistent ETag for empty content
48-
return "\"d41d8cd98f00b204e9800998ecf8427e\"";
49+
// Return a consistent ETag for empty content (SHA256 hash of empty payload).
50+
return $"\"{EmptyContentHash}\"";
4951
}
5052

5153
// Compute SHA256 hash
@@ -122,13 +124,12 @@ public bool IsModified(string? ifNoneMatch, string currentETag)
122124
// Parse multiple ETags (comma-separated)
123125
var etags = ifNoneMatch.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
124126

127+
var current = ParseETag(currentETag);
125128
foreach (var etag in etags)
126129
{
127-
// Normalize ETags for comparison (remove quotes if present)
128-
var normalizedIfNoneMatch = NormalizeETag(etag);
129-
var normalizedCurrent = NormalizeETag(currentETag);
130-
131-
if (string.Equals(normalizedIfNoneMatch, normalizedCurrent, StringComparison.Ordinal))
130+
// If-None-Match uses weak comparison for GET/HEAD.
131+
var candidate = ParseETag(etag);
132+
if (string.Equals(candidate.Value, current.Value, StringComparison.Ordinal))
132133
{
133134
return false; // ETag matches, resource not modified
134135
}
@@ -161,13 +162,17 @@ public bool MatchesPrecondition(string? ifMatch, string currentETag)
161162
// Parse multiple ETags (comma-separated)
162163
var etags = ifMatch.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
163164

165+
var current = ParseETag(currentETag);
164166
foreach (var etag in etags)
165167
{
166-
// Normalize ETags for comparison (remove quotes if present)
167-
var normalizedIfMatch = NormalizeETag(etag);
168-
var normalizedCurrent = NormalizeETag(currentETag);
168+
// If-Match requires strong comparison; weak tags never match.
169+
var candidate = ParseETag(etag);
170+
if (candidate.IsWeak || current.IsWeak)
171+
{
172+
continue;
173+
}
169174

170-
if (string.Equals(normalizedIfMatch, normalizedCurrent, StringComparison.Ordinal))
175+
if (string.Equals(candidate.Value, current.Value, StringComparison.Ordinal))
171176
{
172177
return true; // ETag matches, precondition passes
173178
}
@@ -203,26 +208,27 @@ public void SetCacheHeaders(HttpResponse response, string etag, DateTimeOffset?
203208
}
204209
}
205210

206-
/// <summary>
207-
/// Normalizes an ETag by removing surrounding quotes and whitespace.
208-
/// </summary>
209-
/// <param name="etag">The ETag to normalize</param>
210-
/// <returns>The normalized ETag without quotes</returns>
211-
private static string NormalizeETag(string etag)
211+
private static (string Value, bool IsWeak) ParseETag(string etag)
212212
{
213-
if (string.IsNullOrEmpty(etag))
213+
if (string.IsNullOrWhiteSpace(etag))
214214
{
215-
return string.Empty;
215+
return (string.Empty, false);
216216
}
217217

218218
var trimmed = etag.Trim();
219+
var isWeak = false;
220+
221+
if (trimmed.StartsWith("W/", StringComparison.OrdinalIgnoreCase))
222+
{
223+
isWeak = true;
224+
trimmed = trimmed[2..].TrimStart();
225+
}
219226

220-
// Remove quotes if present
221227
if (trimmed.StartsWith('"') && trimmed.EndsWith('"') && trimmed.Length >= 2)
222228
{
223-
return trimmed[1..^1];
229+
trimmed = trimmed[1..^1];
224230
}
225231

226-
return trimmed;
232+
return (trimmed, isWeak);
227233
}
228234
}

0 commit comments

Comments
 (0)