fix(dav): Improve handling and logging of bulk upload failures

Signed-off-by: Joas Schilling <coding@schilljs.com>

apps/dav/lib/BulkUpload/BulkUploadPlugin.php

@@ -65,7 +65,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
  return true;
  }
 
- $multiPartParser = new MultipartRequestParser($request);
+ $multiPartParser = new MultipartRequestParser($request, $this->logger);
  $writtenFiles = [];
 
  while (!$multiPartParser->isAtLastBoundary()) {

apps/dav/lib/BulkUpload/MultipartRequestParser.php

@@ -23,6 +23,7 @@
 namespace OCA\DAV\BulkUpload;
 
 use OCP\AppFramework\Http;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Exception;
 use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\LengthRequired;
@@ -42,7 +43,10 @@ class MultipartRequestParser {
  /**
  * @throws BadRequest
  */
- public function __construct(RequestInterface $request) {
+ public function __construct(
+ RequestInterface $request,
+ protected LoggerInterface $logger,
+ ) {
  $stream = $request->getBody();
  $contentType = $request->getHeader('Content-Type');
 
@@ -78,7 +82,7 @@ private function parseBoundaryFromHeaders(string $contentType): string {
  $boundaryValue = trim($boundaryValue);
 
  // Remove potential quotes around boundary value.
- if (substr($boundaryValue, 0, 1) == '"' && substr($boundaryValue, -1) == '"') {
+ if (substr($boundaryValue, 0, 1) === '"' && substr($boundaryValue, -1) === '"') {
  $boundaryValue = substr($boundaryValue, 1, -1);
  }
 
@@ -179,6 +183,11 @@ private function readPartHeaders(): array {
  throw new Exception('An error occurred while reading headers of a part');
  }
 
+ if (!str_contains($line, ':')) {
+ $this->logger->error('Header missing `:` on bulk request: "' . json_encode($line) . '"');
+ throw new Exception('An error occurred while reading headers of a part', Http::STATUS_BAD_REQUEST);
+ }
+
  try {
  [$key, $value] = explode(':', $line, 2);
  $headers[strtolower(trim($key))] = trim($value);

apps/dav/tests/unit/Files/MultipartRequestParserTest.php

@@ -23,9 +23,17 @@
 namespace OCA\DAV\Tests\unit\DAV;
 
 use \OCA\DAV\BulkUpload\MultipartRequestParser;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class MultipartRequestParserTest extends TestCase {
+
+ protected LoggerInterface $logger;
+
+ protected function setUp(): void {
+ $this->logger = $this->createMock(LoggerInterface::class);
+ }
+
  private function getValidBodyObject() {
  return [
  [
@@ -73,7 +81,7 @@ private function getMultipartParser(array $parts, array $headers = [], string $b
  ->method('getBody')
  ->willReturn($stream);
 
- return new MultipartRequestParser($request);
+ return new MultipartRequestParser($request, $this->logger);
  }
 
 
@@ -90,7 +98,7 @@ public function testBodyTypeValidation(): void {
  ->willReturn($bodyStream);
 
  $this->expectExceptionMessage('Body should be of type resource');
- new MultipartRequestParser($request);
+ new MultipartRequestParser($request, $this->logger);
  }
 
  /**