diff --git a/pkgs/google_cloud/CHANGELOG.md b/pkgs/google_cloud/CHANGELOG.md index af42a014..aecfc334 100644 --- a/pkgs/google_cloud/CHANGELOG.md +++ b/pkgs/google_cloud/CHANGELOG.md @@ -1,25 +1,35 @@ -## 0.4.1-wip +## 0.4.1 -### `bad_request_exception.dart` +### `http_serving.dart` -- Expanded `BadRequestException` to support structured error reporting as +- Introduces `HttpResponseException` and `httpResponseExceptionMiddleware`. + - `BadRequestException` and `badRequestMiddleware` are deprecated in favor of + `HttpResponseException` and `httpResponseExceptionMiddleware`, respectively. + (They are aliased for backward compatibility.) +- Expanded `HttpResponseException` to support structured error reporting as per AIP-193. - Added `status` (`String?`) and `details` (`List>?`) fields. - Added `toJson()` method to serialize the error into a standard Google Cloud error payload. - Updated `toString()` to include `status` and `details` when non-null. - - Added factory constructors for common HTTP 4XX status codes: `badRequest` - (400), `unauthorized` (401), `forbidden` (403), `notFound` (404), - `conflict` (409), and `tooManyRequests` (429). + - Added factory constructors for common HTTP 4XX and 5XX status codes: + - `badRequest` (400) + - `unauthorized` (401) + - `forbidden` (403) + - `notFound` (404) + - `conflict` (409) + - `tooManyRequests` (429) + - `internalServerError` (500) + - `notImplemented` (501) + - `serviceUnavailable` (503) + - `gatewayTimeout` (504) - Added default `status` values for factories that map 1:1 to gRPC status codes (e.g., `unauthorized` defaults to `'UNAUTHENTICATED'`). - -### `http_serving.dart` (and internal files) - -- Updated `badRequestMiddleware` to leverage `BadRequestException.toJson()` for - JSON responses, returning a standard Google Cloud error payload. -- Updated plain text errors to use `BadRequestException.toString()`. +- Updated `httpResponseExceptionMiddleware` to leverage + `HttpResponseException.toJson()` for JSON responses, returning a standard + Google Cloud error payload. +- Updated plain text errors to use `HttpResponseException.toString()`. ## 0.4.0 diff --git a/pkgs/google_cloud/example/example.dart b/pkgs/google_cloud/example/example.dart index c4082ad8..ab5e5a83 100644 --- a/pkgs/google_cloud/example/example.dart +++ b/pkgs/google_cloud/example/example.dart @@ -87,7 +87,7 @@ Handler _onlyGetRootMiddleware(Handler handler) => (Request request) async { return await handler(request); } - throw BadRequestException(404, 'Not found'); + throw HttpResponseException.notFound(); }; CommitRequest _incrementRequest(String projectId) => CommitRequest( diff --git a/pkgs/google_cloud/lib/general.dart b/pkgs/google_cloud/lib/general.dart index b09fa5f8..16553936 100644 --- a/pkgs/google_cloud/lib/general.dart +++ b/pkgs/google_cloud/lib/general.dart @@ -14,18 +14,20 @@ /// General Google Cloud Platform integration features. /// -/// {@canonicalFor gcp_project.computeProjectId} -/// {@canonicalFor gcp_project.projectIdFromCredentialsFile} -/// {@canonicalFor gcp_project.projectIdFromEnvironmentVariables} -/// {@canonicalFor gcp_project.projectIdFromGcloudConfig} -/// {@canonicalFor gcp_project.MetadataServerException} -/// {@canonicalFor gcp_project.projectIdFromMetadataServer} -/// {@canonicalFor gcp_project.serviceAccountEmailFromMetadataServer} -/// {@canonicalFor logger.LogSeverity} -/// {@canonicalFor logger.CloudLogger} -/// {@canonicalFor structured_logging.structuredLogEntry} -/// {@canonicalFor metadata.gceMetadataHost} -/// {@canonicalFor metadata.gceMetadataUrl} +/// {@canonicalFor general.CloudLogger} +/// {@canonicalFor general.computeProjectId} +/// {@canonicalFor general.fetchMetadataValue} +/// {@canonicalFor general.gceMetadataHost} +/// {@canonicalFor general.gceMetadataUrl} +/// {@canonicalFor general.getMetadataValue} +/// {@canonicalFor general.LogSeverity} +/// {@canonicalFor general.MetadataServerException} +/// {@canonicalFor general.projectIdFromCredentialsFile} +/// {@canonicalFor general.projectIdFromEnvironmentVariables} +/// {@canonicalFor general.projectIdFromGcloudConfig} +/// {@canonicalFor general.projectIdFromMetadataServer} +/// {@canonicalFor general.serviceAccountEmailFromMetadataServer} +/// {@canonicalFor general.structuredLogEntry} library; export 'src/gcp_project.dart' diff --git a/pkgs/google_cloud/lib/http_serving.dart b/pkgs/google_cloud/lib/http_serving.dart index db3b2a3f..b0233590 100644 --- a/pkgs/google_cloud/lib/http_serving.dart +++ b/pkgs/google_cloud/lib/http_serving.dart @@ -38,27 +38,35 @@ /// } /// ``` /// -/// {@canonicalFor bad_configuration_exception.BadConfigurationException} -/// {@canonicalFor bad_request_exception.BadRequestException} -/// {@canonicalFor http_logging.badRequestMiddleware} -/// {@canonicalFor http_logging.cloudLoggingMiddleware} -/// {@canonicalFor http_logging.createLoggingMiddleware} -/// {@canonicalFor http_logging.currentLogger} -/// {@canonicalFor serve.listenPortFromEnvironment} -/// {@canonicalFor serve.serveHandler} -/// {@canonicalFor terminate.waitForTerminate} +/// {@canonicalFor http_serving.BadConfigurationException} +/// {@canonicalFor http_serving.badRequestMiddleware} +/// {@canonicalFor http_serving.cloudLoggingMiddleware} +/// {@canonicalFor http_serving.createLoggingMiddleware} +/// {@canonicalFor http_serving.currentLogger} +/// {@canonicalFor http_serving.httpResponseExceptionMiddleware} +/// {@canonicalFor http_serving.BadRequestException} +/// {@canonicalFor http_serving.HttpResponseException} +/// {@canonicalFor http_serving.listenPortFromEnvironment} +/// {@canonicalFor http_serving.serveHandler} +/// {@canonicalFor http_serving.TraceContextData} +/// {@canonicalFor http_serving.waitForTerminate} library; export 'src/logger.dart' show CloudLogger, LogSeverity; export 'src/serving/bad_configuration_exception.dart' show BadConfigurationException; -export 'src/serving/bad_request_exception.dart' show BadRequestException; export 'src/serving/http_logging.dart' show badRequestMiddleware, cloudLoggingMiddleware, createLoggingMiddleware, - currentLogger; + currentLogger, + httpResponseExceptionMiddleware; +export 'src/serving/http_response_exception.dart' + show + // ignore: deprecated_member_use_from_same_package + BadRequestException, + HttpResponseException; export 'src/serving/serve.dart' show listenPortFromEnvironment, serveHandler; export 'src/serving/terminate.dart' show waitForTerminate; export 'src/serving/trace_context_data.dart' show TraceContextData; diff --git a/pkgs/google_cloud/lib/src/serving/http_logging.dart b/pkgs/google_cloud/lib/src/serving/http_logging.dart index 6d6cc00c..b7ee48af 100644 --- a/pkgs/google_cloud/lib/src/serving/http_logging.dart +++ b/pkgs/google_cloud/lib/src/serving/http_logging.dart @@ -22,13 +22,14 @@ import 'package:shelf/shelf.dart'; import '../constants.dart'; import '../logger.dart'; import '../structured_logging.dart'; -import 'bad_request_exception.dart'; +import 'http_response_exception.dart'; +import 'json_request_checking.dart'; import 'trace_context_data.dart'; export '../structured_logging.dart'; -const _badRequestExceptionContextKey = 'google_cloud.bad_request_exception'; -const _badStackTraceContextKey = 'google_cloud.bad_stack_trace'; +const _httpResponseExceptionKey = 'google_cloud.response_exception'; +const _exceptionStackTraceKey = 'google_cloud.bad_stack_trace'; /// Convenience [Middleware] that handles logging depending on [projectId]. /// @@ -36,23 +37,28 @@ const _badStackTraceContextKey = 'google_cloud.bad_stack_trace'; /// correlation. /// /// If [projectId] is `null`, returns [Middleware] composed of [logRequests] and -/// [badRequestMiddleware]. +/// [httpResponseExceptionMiddleware]. /// /// If [projectId] is provided, returns the value from [cloudLoggingMiddleware]. Middleware createLoggingMiddleware({String? projectId}) => projectId == null - ? _errorWriter.addMiddleware(logRequests()).addMiddleware(_handleBadRequest) + ? _errorWriter + .addMiddleware(logRequests()) + .addMiddleware(_handleResponseException) : cloudLoggingMiddleware(projectId); -/// Adds logic which catches [BadRequestException], logs details to [stderr] and -/// returns a corresponding [Response]. -Middleware get badRequestMiddleware => _handleBadRequest; +@Deprecated('use httpResponseExceptionMiddleware instead') +Middleware get badRequestMiddleware => httpResponseExceptionMiddleware; -Handler _handleBadRequest(Handler innerHandler) => (request) async { +/// Adds logic which catches [HttpResponseException], logs details to [stderr] +/// and returns a corresponding [Response]. +Middleware get httpResponseExceptionMiddleware => _handleResponseException; + +Handler _handleResponseException(Handler innerHandler) => (request) async { try { final response = await innerHandler(request); return response; - } on BadRequestException catch (error, stack) { - return _responseFromBadRequest(error, stack, request.headers); + } on HttpResponseException catch (error, stack) { + return _responseFromException(error, stack, request.headers); } }; @@ -60,10 +66,10 @@ Handler _errorWriter(Handler innerHandler) => (request) async { final response = await innerHandler(request); final error = - response.context[_badRequestExceptionContextKey] as BadRequestException?; + response.context[_httpResponseExceptionKey] as HttpResponseException?; if (error != null) { - final stack = response.context[_badStackTraceContextKey] as StackTrace?; + final stack = response.context[_exceptionStackTraceKey] as StackTrace?; final output = [ error, if (error.innerError != null) @@ -80,41 +86,27 @@ Handler _errorWriter(Handler innerHandler) => (request) async { return response; }; -Response _responseFromBadRequest( - BadRequestException e, +Response _responseFromException( + HttpResponseException e, StackTrace stack, [ Map? requestHeaders, ]) { - if (_isJsonRequest(requestHeaders)) { + if (requestHeaders != null && shouldSendJsonResponse(requestHeaders)) { return Response( e.statusCode, body: jsonEncode(e.toJson()), - headers: {HttpHeaders.contentTypeHeader: _jsonMimeType}, - context: { - _badRequestExceptionContextKey: e, - _badStackTraceContextKey: stack, - }, + headers: {HttpHeaders.contentTypeHeader: 'application/json'}, + context: {_httpResponseExceptionKey: e, _exceptionStackTraceKey: stack}, ); } return Response( e.statusCode, body: e.toString(), - context: { - _badRequestExceptionContextKey: e, - _badStackTraceContextKey: stack, - }, + context: {_httpResponseExceptionKey: e, _exceptionStackTraceKey: stack}, ); } -const _jsonMimeType = 'application/json'; - -bool _isJsonRequest(Map? headers) { - final accept = headers?[HttpHeaders.acceptHeader] ?? ''; - final contentType = headers?[HttpHeaders.contentTypeHeader] ?? ''; - return accept.contains(_jsonMimeType) || contentType.contains(_jsonMimeType); -} - /// Return [Middleware] that logs errors using Google Cloud structured logs and /// returns the correct response. /// @@ -138,16 +130,66 @@ Middleware cloudLoggingMiddleware(String projectId) { String createErrorLogEntryFromRequest( Object error, StackTrace? stackTrace, - LogSeverity logSeverity, - ) => structuredLogEntry( + LogSeverity logSeverity, { + Map? extraPayload, + }) => structuredLogEntry( '$error'.trim(), logSeverity, - payload: traceContext?.asPayloadMap(), + payload: {...?traceContext?.asPayloadMap(), ...?extraPayload}, stackTrace: stackTrace, ); final completer = Completer.sync(); + void uncaughtErrorHandler( + Zone self, + ZoneDelegate parent, + _, + Object error, + StackTrace stackTrace, + ) { + if (error is HijackException) { + completer.completeError(error, stackTrace); + } + + final logContentString = error is HttpResponseException + ? createErrorLogEntryFromRequest( + error, + error.innerStack ?? stackTrace, + error.statusCode >= 500 ? LogSeverity.error : LogSeverity.warning, + extraPayload: error.toJson(), + ) + : createErrorLogEntryFromRequest( + error, + stackTrace, + LogSeverity.error, + ); + + // Serialize to a JSON string and output. + parent.print(self, logContentString); + + if (completer.isCompleted) { + return; + } + + final response = error is HttpResponseException + ? _responseFromException(error, stackTrace, request.headers) + : Response.internalServerError(); + + completer.complete(response); + } + + void zonePrint(Zone self, ZoneDelegate parent, _, String line) { + final logContent = structuredLogEntry( + line, + LogSeverity.info, + payload: traceContext?.asPayloadMap(), + ); + + // Serialize to a JSON string and output to parent zone. + parent.print(self, logContent); + } + final currentZone = Zone.current; Zone.current @@ -159,46 +201,8 @@ Middleware cloudLoggingMiddleware(String projectId) { ), }, specification: ZoneSpecification( - handleUncaughtError: (self, parent, zone, error, stackTrace) { - if (error is HijackException) { - completer.completeError(error, stackTrace); - } - - final logContentString = error is BadRequestException - ? createErrorLogEntryFromRequest( - 'Bad request. ${error.message}', - error.innerStack ?? stackTrace, - LogSeverity.warning, - ) - : createErrorLogEntryFromRequest( - error, - stackTrace, - LogSeverity.error, - ); - - // Serialize to a JSON string and output. - parent.print(self, logContentString); - - if (completer.isCompleted) { - return; - } - - final response = error is BadRequestException - ? _responseFromBadRequest(error, stackTrace, request.headers) - : Response.internalServerError(); - - completer.complete(response); - }, - print: (self, parent, zone, line) { - final logContent = structuredLogEntry( - line, - LogSeverity.info, - payload: traceContext?.asPayloadMap(), - ); - - // Serialize to a JSON string and output to parent zone. - parent.print(self, logContent); - }, + handleUncaughtError: uncaughtErrorHandler, + print: zonePrint, ), ) .runGuarded(() async { diff --git a/pkgs/google_cloud/lib/src/serving/bad_request_exception.dart b/pkgs/google_cloud/lib/src/serving/http_response_exception.dart similarity index 54% rename from pkgs/google_cloud/lib/src/serving/bad_request_exception.dart rename to pkgs/google_cloud/lib/src/serving/http_response_exception.dart index cfeec3b8..a2835237 100644 --- a/pkgs/google_cloud/lib/src/serving/bad_request_exception.dart +++ b/pkgs/google_cloud/lib/src/serving/http_response_exception.dart @@ -16,20 +16,24 @@ import 'package:shelf/shelf.dart'; import 'http_logging.dart'; -/// When thrown in a [Handler], configured with [badRequestMiddleware] or -/// similar, causes a response with [statusCode] to be returned. +@Deprecated('use HttpResponseException instead') +typedef BadRequestException = HttpResponseException; + +/// When thrown in a [Handler], configured with +/// [httpResponseExceptionMiddleware] or similar, causes a response with +/// [statusCode] to be returned. /// -/// [statusCode] must be >= `400` and <= `499`. +/// [statusCode] must be >= `400` and <= `599`. /// /// [message] is used as the body of the response send to the requester. /// /// If provided, [innerError] and [innerStack] can be used to provide additional /// debugging information which is included in logs, but not sent to the /// requester. -class BadRequestException implements Exception { +class HttpResponseException implements Exception { /// The HTTP status code for the response. /// - /// Must be between 400 and 499. + /// Must be between 400 and 599. final int statusCode; /// The message sent to the requester. @@ -51,7 +55,7 @@ class BadRequestException implements Exception { /// See https://google.aip.dev/193#statusdetails final List>? details; - BadRequestException( + HttpResponseException( this.statusCode, this.message, { this.innerError, @@ -59,51 +63,55 @@ class BadRequestException implements Exception { this.status, this.details, }) : assert(message.isNotEmpty) { - if (statusCode < 400 || statusCode > 499) { + if (statusCode < 400 || statusCode > 599) { throw ArgumentError.value( statusCode, 'statusCode', - 'Must be between 400 and 499', + 'Must be between 400 and 599', ); } } - /// Creates a new [BadRequestException] with status code 400. - factory BadRequestException.badRequest( - String message, { + /// Creates a new [HttpResponseException] with status code 400. + /// + /// [message] defaults to `'Bad Request'`. + factory HttpResponseException.badRequest({ + String? message, Object? innerError, StackTrace? innerStack, - String? status, + String? status = 'INVALID_ARGUMENT', List>? details, - }) => BadRequestException( + }) => HttpResponseException( 400, - message, + message ?? 'Bad Request', innerError: innerError, innerStack: innerStack, status: status, details: details, ); - /// Creates a new [BadRequestException] with status code 401. + /// Creates a new [HttpResponseException] with status code 401. /// /// The request does not have valid authentication credentials for the /// operation. - factory BadRequestException.unauthorized( - String message, { + /// + /// [message] defaults to `'Unauthorized'`. + factory HttpResponseException.unauthorized({ + String? message, Object? innerError, StackTrace? innerStack, String? status = 'UNAUTHENTICATED', List>? details, - }) => BadRequestException( + }) => HttpResponseException( 401, - message, + message ?? 'Unauthorized', innerError: innerError, innerStack: innerStack, status: status, details: details, ); - /// Creates a new [BadRequestException] with status code 403. + /// Creates a new [HttpResponseException] with status code 403. /// /// The caller does not have permission to execute the specified /// operation. `PERMISSION_DENIED` must not be used for rejections @@ -113,22 +121,24 @@ class BadRequestException implements Exception { /// instead for those errors). This error code does not imply the /// request is valid or the requested entity exists or satisfies /// other pre-conditions. - factory BadRequestException.forbidden( - String message, { + /// + /// [message] defaults to `'Forbidden'`. + factory HttpResponseException.forbidden({ + String? message, Object? innerError, StackTrace? innerStack, String? status = 'PERMISSION_DENIED', List>? details, - }) => BadRequestException( + }) => HttpResponseException( 403, - message, + message ?? 'Forbidden', innerError: innerError, innerStack: innerStack, status: status, details: details, ); - /// Creates a new [BadRequestException] with status code 404. + /// Creates a new [HttpResponseException] with status code 404. /// /// Some requested entity (e.g., file or directory) was not found. /// @@ -137,50 +147,128 @@ class BadRequestException implements Exception { /// `NOT_FOUND` may be used. If a request is denied for some users within /// a class of users, such as user-based access control, `PERMISSION_DENIED` /// must be used. - factory BadRequestException.notFound( - String message, { + /// + /// [message] defaults to `'Not Found'`. + factory HttpResponseException.notFound({ + String? message, Object? innerError, StackTrace? innerStack, String? status = 'NOT_FOUND', List>? details, - }) => BadRequestException( + }) => HttpResponseException( 404, - message, + message ?? 'Not Found', innerError: innerError, innerStack: innerStack, status: status, details: details, ); - /// Creates a new [BadRequestException] with status code 409. - factory BadRequestException.conflict( - String message, { + /// Creates a new [HttpResponseException] with status code 409. + /// + /// [message] defaults to `'Conflict'`. + factory HttpResponseException.conflict({ + String? message, Object? innerError, StackTrace? innerStack, - String? status, + String? status = 'ALREADY_EXISTS', List>? details, - }) => BadRequestException( + }) => HttpResponseException( 409, - message, + message ?? 'Conflict', innerError: innerError, innerStack: innerStack, status: status, details: details, ); - /// Creates a new [BadRequestException] with status code 429. + /// Creates a new [HttpResponseException] with status code 429. /// /// Some resource has been exhausted, perhaps a per-user quota, or /// perhaps the entire file system is out of space. - factory BadRequestException.tooManyRequests( - String message, { + /// + /// [message] defaults to `'Too Many Requests'`. + factory HttpResponseException.tooManyRequests({ + String? message, Object? innerError, StackTrace? innerStack, String? status = 'RESOURCE_EXHAUSTED', List>? details, - }) => BadRequestException( + }) => HttpResponseException( 429, - message, + message ?? 'Too Many Requests', + innerError: innerError, + innerStack: innerStack, + status: status, + details: details, + ); + + /// Creates a new [HttpResponseException] with status code 500. + /// + /// [message] defaults to `'Internal Server Error'`. + factory HttpResponseException.internalServerError({ + String? message, + Object? innerError, + StackTrace? innerStack, + String? status = 'INTERNAL', + List>? details, + }) => HttpResponseException( + 500, + message ?? 'Internal Server Error', + innerError: innerError, + innerStack: innerStack, + status: status, + details: details, + ); + + /// Creates a new [HttpResponseException] with status code 501. + /// + /// [message] defaults to `'Not Implemented'`. + factory HttpResponseException.notImplemented({ + String? message, + Object? innerError, + StackTrace? innerStack, + String? status = 'UNIMPLEMENTED', + List>? details, + }) => HttpResponseException( + 501, + message ?? 'Not Implemented', + innerError: innerError, + innerStack: innerStack, + status: status, + details: details, + ); + + /// Creates a new [HttpResponseException] with status code 503. + /// + /// [message] defaults to `'Service Unavailable'`. + factory HttpResponseException.serviceUnavailable({ + String? message, + Object? innerError, + StackTrace? innerStack, + String? status = 'UNAVAILABLE', + List>? details, + }) => HttpResponseException( + 503, + message ?? 'Service Unavailable', + innerError: innerError, + innerStack: innerStack, + status: status, + details: details, + ); + + /// Creates a new [HttpResponseException] with status code 504. + /// + /// [message] defaults to `'Gateway Timeout'`. + factory HttpResponseException.gatewayTimeout({ + String? message, + Object? innerError, + StackTrace? innerStack, + String? status = 'DEADLINE_EXCEEDED', + List>? details, + }) => HttpResponseException( + 504, + message ?? 'Gateway Timeout', innerError: innerError, innerStack: innerStack, status: status, diff --git a/pkgs/google_cloud/lib/src/serving/json_request_checking.dart b/pkgs/google_cloud/lib/src/serving/json_request_checking.dart new file mode 100644 index 00000000..37003bdb --- /dev/null +++ b/pkgs/google_cloud/lib/src/serving/json_request_checking.dart @@ -0,0 +1,94 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'dart:io'; +import 'package:http/http.dart' show MediaType; +import 'package:meta/meta.dart'; + +/// Evaluates whether to send a JSON response based on request headers. +/// +/// Rules: +/// 1. Return `true` if the `Accept` header explicitly asks for JSON +/// (with q > 0). +/// 2. Return `false` if the `Accept` header explicitly forbids JSON (q = 0) +/// and does not allow it elsewhere. +/// 3. Fallback to return `true` if the `Content-Type` header is any +/// valid flavor of JSON when the `Accept` header does not specify +/// JSON preference. +/// +/// This is a simplified content negotiation that prefers JSON if allowed, +/// or falls back to the request Content-Type. +@internal +bool shouldSendJsonResponse(Map requestHeaders) { + final accept = requestHeaders[HttpHeaders.acceptHeader]; + + var jsonExplicitlyAllowed = false; + var jsonExplicitlyForbidden = false; + + if (accept != null) { + // TODO(kevmoo): leverage support in pkg:http_parser when it lands + // https://github.com/dart-lang/http/issues/1904 + + final values = accept.split(','); + for (final value in values) { + final trimmed = value.trim(); + final MediaType mediaType; + try { + mediaType = MediaType.parse(trimmed); + } catch (_) { + // Ignore invalid media types in Accept header + continue; + } + + final q = double.tryParse(mediaType.parameters['q'] ?? '1.0') ?? 1.0; + + if (_isJson(mediaType)) { + if (q > 0.0) { + jsonExplicitlyAllowed = true; + } else { + jsonExplicitlyForbidden = true; + } + } + } + } + + // If explicitly forbidden and not allowed by another spec, return false. + final forbidsJson = jsonExplicitlyForbidden && !jsonExplicitlyAllowed; + if (forbidsJson) return false; + + // If explicitly allowed, return true. + if (jsonExplicitlyAllowed) return true; + + final contentType = requestHeaders[HttpHeaders.contentTypeHeader]; + + // Fallback to Content-Type if it is any valid flavor of JSON. + if (contentType != null && _isAnyJson(contentType)) { + return true; + } + + return false; +} + +bool _isAnyJson(String contentTypeHeader) { + try { + final mediaType = MediaType.parse(contentTypeHeader); + return _isJson(mediaType); + } catch (e) { + return false; + } +} + +bool _isJson(MediaType mediaType) => + mediaType.mimeType == 'application/json' || + mediaType.subtype.endsWith('+json'); diff --git a/pkgs/google_cloud/lib/src/serving/terminate.dart b/pkgs/google_cloud/lib/src/serving/terminate.dart index a2763960..81b2904a 100644 --- a/pkgs/google_cloud/lib/src/serving/terminate.dart +++ b/pkgs/google_cloud/lib/src/serving/terminate.dart @@ -25,7 +25,6 @@ Future waitForTerminate() { final completer = Completer.sync(); // sigIntSub is copied below to avoid a race condition - ignoring this lint - // ignore: cancel_subscriptions StreamSubscription? sigIntSub, sigTermSub; Future signalHandler(ProcessSignal signal) async { diff --git a/pkgs/google_cloud/pubspec.yaml b/pkgs/google_cloud/pubspec.yaml index 3ad432b2..be80b847 100644 --- a/pkgs/google_cloud/pubspec.yaml +++ b/pkgs/google_cloud/pubspec.yaml @@ -1,7 +1,7 @@ name: google_cloud description: >- Utilities for running Dart code correctly on the Google Cloud Platform. -version: 0.4.0 +version: 0.4.1 repository: https://github.com/googleapis/google-cloud-dart/tree/main/pkgs/google_cloud resolution: workspace diff --git a/pkgs/google_cloud/test/json_request_checking_test.dart b/pkgs/google_cloud/test/json_request_checking_test.dart new file mode 100644 index 00000000..5477932c --- /dev/null +++ b/pkgs/google_cloud/test/json_request_checking_test.dart @@ -0,0 +1,152 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'dart:io'; +import 'package:google_cloud/src/serving/json_request_checking.dart'; +import 'package:test/test.dart'; + +void main() { + group('shouldSendJsonResponse', () { + test('empty headers returns false', () { + expect(shouldSendJsonResponse({}), isFalse); + }); + + test('Accept: application/json returns true', () { + expect( + shouldSendJsonResponse({HttpHeaders.acceptHeader: 'application/json'}), + isTrue, + ); + }); + + test('Accept: text/plain returns false', () { + expect( + shouldSendJsonResponse({HttpHeaders.acceptHeader: 'text/plain'}), + isFalse, + ); + }); + + test('Accept: application/json;q=0.5, text/plain;q=0.8 returns true ' + '(prefers JSON if allowed)', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: 'application/json;q=0.5, text/plain;q=0.8', + }), + isTrue, + ); + }); + + test('Accept: application/json;q=0.8, text/plain;q=0.5 returns true', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: 'application/json;q=0.8, text/plain;q=0.5', + }), + isTrue, + ); + }); + + test('Accept: application/json;q=0 returns false (explicitly forbids)', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: 'application/json;q=0', + }), + isFalse, + ); + }); + + test('Content-Type: application/json returns true', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.contentTypeHeader: 'application/json', + }), + isTrue, + ); + }); + + test('Content-Type: application/problem+json returns true', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.contentTypeHeader: 'application/problem+json', + }), + isTrue, + ); + }); + + test('Conflict: Content-Type JSON but Accept forbids returns false', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.contentTypeHeader: 'application/json', + HttpHeaders.acceptHeader: 'application/json;q=0', + }), + isFalse, + ); + }); + + test('Accept with invalid media type is ignored', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: 'invalid-mime, application/json', + }), + isTrue, + ); + }); + + test('Accept with JSON and text equal priority returns true ' + '(prefers JSON)', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: 'application/json, text/plain', + }), + isTrue, + ); + }); + + test('Accept: */* returns false (defaults to text)', () { + expect( + shouldSendJsonResponse({HttpHeaders.acceptHeader: '*/*'}), + isFalse, + ); + }); + + test('Accept with multiple JSON specs (one allowed, one forbidden) ' + 'returns true', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: + 'application/json, application/manifest+json;q=0', + }), + isTrue, + ); + }); + + test('Accept with multiple JSON specs (forbidden first) returns true', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: + 'application/manifest+json;q=0, application/json', + }), + isTrue, + ); + }); + + test('testing mixed json bits', () { + expect( + shouldSendJsonResponse({ + HttpHeaders.acceptHeader: + 'application/json; q=1, application/ld+json; q=0', + }), + isTrue, + ); + }); + }); +} diff --git a/pkgs/google_cloud/test/logging_test.dart b/pkgs/google_cloud/test/logging_test.dart index 73bd4fd4..7b348129 100644 --- a/pkgs/google_cloud/test/logging_test.dart +++ b/pkgs/google_cloud/test/logging_test.dart @@ -193,12 +193,10 @@ void main() { group('middleware', () { test('cloudLoggingMiddleware logs structured entries', () async { - final handler = const Pipeline() - .addMiddleware(cloudLoggingMiddleware('test-project')) - .addHandler((request) { - currentLogger.info('inner log'); - return Response.ok('done'); - }); + final handler = _testHandler((request) { + currentLogger.info('inner log'); + return Response.ok('done'); + }); await expectLater( () => handler( @@ -212,127 +210,261 @@ void main() { ), ), prints( - predicate((output) { - final map = jsonDecode(output) as Map; - return map['message'] == 'inner log' && - map['severity'] == 'INFO' && - map['logging.googleapis.com/trace'] == - 'projects/test-project/traces/0123456789abcdef0123456789abcdef' && - map['logging.googleapis.com/spanId'] == '000000000000007b' && - map['logging.googleapis.com/trace_sampled'] == true; - }), + isA().having( + (output) => jsonDecode(output) as Map, + 'as map', + { + 'message': 'inner log', + 'severity': 'INFO', + 'logging.googleapis.com/trace': + 'projects/test-project/traces/0123456789abcdef0123456789abcdef', + 'logging.googleapis.com/spanId': '000000000000007b', + 'logging.googleapis.com/trace_sampled': true, + }, + ), ), ); }); - test('badRequestMiddleware handles BadRequestException', () async { - final handler = const Pipeline() - .addMiddleware(badRequestMiddleware) - .addHandler((request) { - throw BadRequestException(400, 'Custom bad request'); - }); + test( + 'cloudLoggingMiddleware logs HttpResponseException with payload', + () async { + final handler = _testHandler((request) { + throw HttpResponseException( + 403, + 'Forbidden access', + status: 'PERMISSION_DENIED', + ); + }); + + await expectLater( + () => handler(Request('GET', Uri.parse('http://localhost/'))), + prints( + isA().having( + (output) => jsonDecode(output) as Map, + 'as map', + { + 'message': 'Forbidden access (403) [PERMISSION_DENIED]', + 'severity': 'WARNING', + 'error': { + 'code': 403, + 'message': 'Forbidden access', + 'status': 'PERMISSION_DENIED', + }, + 'stack_trace': isA(), + 'logging.googleapis.com/sourceLocation': + isA>(), + }, + ), + ), + ); + }, + ); - final response = await handler( - Request('GET', Uri.parse('http://localhost/')), - ); - expect(response.statusCode, 400); - expect( - await response.readAsString(), - contains('Custom bad request (400)'), // toString() output + test('cloudLoggingMiddleware logs notImplemented with payload', () async { + final handler = _testHandler((request) { + throw HttpResponseException.notImplemented( + message: 'Not implemented yet', + ); + }); + + await expectLater( + () => handler(Request('GET', Uri.parse('http://localhost/'))), + prints( + isA().having( + (output) => jsonDecode(output) as Map, + 'as map', + { + 'message': 'Not implemented yet (501) [UNIMPLEMENTED]', + 'severity': 'ERROR', + 'error': { + 'code': 501, + 'message': 'Not implemented yet', + 'status': 'UNIMPLEMENTED', + }, + 'stack_trace': isA(), + 'logging.googleapis.com/sourceLocation': + isA>(), + }, + ), + ), ); }); - test('badRequestMiddleware handles BadRequestException (JSON)', () async { - final handler = const Pipeline() - .addMiddleware(badRequestMiddleware) - .addHandler((request) { - throw BadRequestException(400, 'Custom bad request'); - }); + test( + 'httpResponseExceptionMiddleware handles HttpResponseException', + () async { + final handler = _testHandler((request) { + throw HttpResponseException(400, 'Custom bad request'); + }); + + late Response response; + await expectLater( + () async => response = await handler( + Request('GET', Uri.parse('http://localhost/')), + ), + prints( + isA().having( + (output) => jsonDecode(output) as Map, + 'as map', + { + 'message': 'Custom bad request (400)', + 'severity': 'WARNING', + 'error': {'code': 400, 'message': 'Custom bad request'}, + 'stack_trace': isA(), + 'logging.googleapis.com/sourceLocation': + isA>(), + }, + ), + ), + ); + expect(response.statusCode, 400); + expect( + await response.readAsString(), + contains('Custom bad request (400)'), // toString() output + ); + }, + ); - final response = await handler( - Request( - 'GET', - Uri.parse('http://localhost/'), - headers: {'Accept': 'application/json'}, - ), - ); - expect(response.statusCode, 400); - expect( - response.headers[HttpHeaders.contentTypeHeader], - contains('application/json'), - ); + test( + 'httpResponseExceptionMiddleware handles HttpResponseException (JSON)', + () async { + final handler = _testHandler((request) { + throw HttpResponseException(400, 'Custom bad request'); + }); + + late Response response; + await expectLater( + () async => response = await handler( + Request( + 'GET', + Uri.parse('http://localhost/'), + headers: {'Accept': 'application/json'}, + ), + ), + prints( + isA().having( + (output) => jsonDecode(output) as Map, + 'as map', + { + 'message': 'Custom bad request (400)', + 'severity': 'WARNING', + 'error': {'code': 400, 'message': 'Custom bad request'}, + 'stack_trace': isA(), + 'logging.googleapis.com/sourceLocation': + isA>(), + }, + ), + ), + ); + expect(response.statusCode, 400); + expect( + response.headers[HttpHeaders.contentTypeHeader], + contains('application/json'), + ); - final body = await response.readAsString(); - final json = jsonDecode(body) as Map; - expect(json, { - 'error': {'code': 400, 'message': 'Custom bad request'}, - }); - }); + final body = await response.readAsString(); + final json = jsonDecode(body) as Map; + expect(json, { + 'error': {'code': 400, 'message': 'Custom bad request'}, + }); + }, + ); test('skips empty details in toJson', () { - final e1 = BadRequestException(400, 'Custom bad request'); + final e1 = HttpResponseException(400, 'Custom bad request'); expect(e1.toJson()['error'], { 'code': 400, 'message': 'Custom bad request', }); - final e2 = BadRequestException(400, 'Custom bad request', details: []); + final e2 = HttpResponseException(400, 'Custom bad request', details: []); expect(e2.toJson()['error'], { 'code': 400, 'message': 'Custom bad request', }); }); - test('badRequestMiddleware handles expanded BadRequestException', () async { - final handler = const Pipeline() - .addMiddleware(badRequestMiddleware) - .addHandler((request) { - throw BadRequestException.badRequest( - 'Custom bad request', - status: 'INVALID_ARGUMENT', - details: [ - {'field': 'name', 'message': 'required'}, - ], - ); - }); + test( + 'httpResponseExceptionMiddleware handles expanded HttpResponseException', + () async { + final handler = _testHandler((request) { + throw HttpResponseException.badRequest( + message: 'Custom bad request', + status: 'INVALID_ARGUMENT', + details: [ + {'field': 'name', 'message': 'required'}, + ], + ); + }); + + late Response response; + await expectLater( + () async => response = await handler( + Request( + 'GET', + Uri.parse('http://localhost/'), + headers: {'Accept': 'application/json'}, + ), + ), + prints( + isA().having( + (output) => jsonDecode(output) as Map, + 'as map', + { + 'message': + 'Custom bad request (400) [INVALID_ARGUMENT] ' + 'Details: [{field: name, message: required}]', + 'severity': 'WARNING', + 'error': { + 'code': 400, + 'message': 'Custom bad request', + 'status': 'INVALID_ARGUMENT', + 'details': [ + {'field': 'name', 'message': 'required'}, + ], + }, + 'stack_trace': isA(), + 'logging.googleapis.com/sourceLocation': + isA>(), + }, + ), + ), + ); + expect(response.statusCode, 400); - final response = await handler( - Request( - 'GET', - Uri.parse('http://localhost/'), - headers: {'Accept': 'application/json'}, - ), - ); - expect(response.statusCode, 400); - - final body = await response.readAsString(); - final json = jsonDecode(body) as Map; - expect(json, { - 'error': { - 'code': 400, - 'message': 'Custom bad request', - 'status': 'INVALID_ARGUMENT', - 'details': [ - {'field': 'name', 'message': 'required'}, - ], - }, - }); - }); + final body = await response.readAsString(); + final json = jsonDecode(body) as Map; + expect(json, { + 'error': { + 'code': 400, + 'message': 'Custom bad request', + 'status': 'INVALID_ARGUMENT', + 'details': [ + {'field': 'name', 'message': 'required'}, + ], + }, + }); + }, + ); - test('badRequestMiddleware logs to stderr', () async { + test('httpResponseExceptionMiddleware logs to stderr', () async { final stderrLines = []; final handler = const Pipeline() .addMiddleware(createLoggingMiddleware()) .addHandler((request) { - throw BadRequestException(400, 'Custom bad request'); + throw HttpResponseException(400, 'Custom bad request'); }); - await IOOverrides.runZoned(() async { - final response = await handler( - Request('GET', Uri.parse('http://localhost/')), - ); - expect(response.statusCode, 400); - }, stderr: () => _MockStdout(stderrLines)); + await expectLater( + () => IOOverrides.runZoned(() async { + final response = await handler( + Request('GET', Uri.parse('http://localhost/')), + ); + expect(response.statusCode, 400); + }, stderr: () => _MockStdout(stderrLines)), + prints(contains('[400] /')), + ); expect(stderrLines, hasLength(1)); final lines = stderrLines.single.split('\n'); @@ -341,26 +473,25 @@ void main() { }); }); - group('BadRequestException', () { + group('HttpResponseException', () { test('valid status code', () { - final ex = BadRequestException(400, 'Bad'); + final ex = HttpResponseException(400, 'Bad'); expect(ex.statusCode, 400); expect(ex.message, 'Bad'); expect(ex.toString(), 'Bad (400)'); }); test('invalid status code low', () { - expect(() => BadRequestException(399, 'Bad'), throwsArgumentError); + expect(() => HttpResponseException(399, 'Bad'), throwsArgumentError); }); test('invalid status code high', () { - expect(() => BadRequestException(500, 'Bad'), throwsArgumentError); + expect(() => HttpResponseException(600, 'Bad'), throwsArgumentError); }); test('empty message', () { - // ignore: prefer_const_constructors expect( - () => BadRequestException(400, ''), + () => HttpResponseException(400, ''), throwsA(isA()), ); }); @@ -472,6 +603,10 @@ void main() { }); } +Handler _testHandler(Handler inner) => const Pipeline() + .addMiddleware(cloudLoggingMiddleware('test-project')) + .addHandler(inner); + class _NonEncodable { @override String toString() => 'I am not encodable';