Remove content length validation to allow for chunked responses#2015
Merged
dkosowski87 merged 8 commits intomainfrom Feb 18, 2026
Merged
Remove content length validation to allow for chunked responses#2015dkosowski87 merged 8 commits intomainfrom
dkosowski87 merged 8 commits intomainfrom
Conversation
* Simplified API calls by removing the `verify_content_length` parameter from `_get_from_url` and related functions. * Updated all instances in the codebase to reflect this change, ensuring consistent behavior across model data retrieval. * Adjusted unit tests to remove unnecessary content length headers in mock responses.
* Added a warning log when the x-goog-hash header is missing during MD5 verification. * Updated the logic to check for the presence of the x-goog-hash header before performing MD5 hash comparison. * Added a unit test to verify behavior when the x-goog-hash header is absent, ensuring proper logging and response handling.
* Updated the logging behavior to ensure that the API key is not included in warning messages when the x-goog-hash header is missing during MD5 verification. * Added a unit test to verify that the API key is not logged in such scenarios, enhancing security and privacy in API interactions.
* Introduced a new header, X-Allow-Chunked, to enable chunked responses from the Roboflow API. * Updated the build_roboflow_api_headers function to include the new header in the request. * Enhanced unit tests to verify the inclusion of the X-Allow-Chunked header in various scenarios.
* Updated error messages in the get_roboflow_workspace and get_roboflow_workspace_async functions to remove unnecessary f-strings, enhancing clarity. * Modified exception handling in load_cached_workflow_response to catch specific exceptions, improving robustness.
dkosowski87
requested review from
PawelPeczek-Roboflow,
grzegorz-roboflow,
hansent,
probicheaux and
yeldarby
as code owners
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if MD5_VERIFICATION_ENABLED: | ||
| if "x-goog-hash" not in response.headers: | ||
| safe_url = API_KEY_PATTERN.sub(deduct_api_key, wrap_url(url)) | ||
| logger.warning( |
Collaborator
There was a problem hiding this comment.
was about dismissing alert - which probably would be fine, but maybe better idea is to urlparse and only provide schema, host and path?
Contributor
Author
There was a problem hiding this comment.
Done that. Because there is more sensitive stuff than just the API_KEY in the params unfortunately.
* Introduced a new utility function, _url_for_safe_logging, to strip sensitive query parameters from URLs before logging. * Updated logging behavior in the MD5 verification process to use the new utility, ensuring sensitive information is not exposed in logs. * Added a unit test to verify that the logged URL path does not include query parameters, enhancing security in API interactions.
* Updated the logic in the _get_from_url function to check for the presence of the md5= part in the x-goog-hash header, adding a warning log if it is missing. * Enhanced unit tests to cover scenarios where the x-goog-hash header is present but lacks the md5= part, ensuring proper logging behavior. * This change improves the clarity of error messages related to MD5 verification, enhancing debugging and monitoring capabilities.
inference/core/roboflow_api.py
Outdated
| md5_part = part.strip()[4:] | ||
| break | ||
| if md5_part is not None: | ||
| md5_from_header = base64.b64decode(md5_part) |
Collaborator
There was a problem hiding this comment.
maybe good idea would be to try except and fail with md5 verification rather than base64 error
grzegorz-roboflow
previously approved these changes
Feb 18, 2026
* Added error handling for invalid base64 values in the x-goog-hash header during MD5 verification in the _get_from_url function. * Introduced a specific exception to raise when the MD5 value is not valid base64, improving clarity in error reporting. * Added a unit test to verify behavior when an invalid MD5 part is encountered, ensuring robust error handling in API interactions.
dkosowski87
requested review from
PawelPeczek-Roboflow and
grzegorz-roboflow
PawelPeczek-Roboflow
approved these changes
Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Due to the
content-lengthvalidation, we couldn't process chunked responses that didn't provide that header. This was evident when compression was introduced inroboflowrepo.Summary
verify_content_lengthparameter from the request helpers and all callers.x-goog-hashheader, or has one but without themd5part in the value a warning is logged with a sanitized request URL (API key redacted).X-Allow-Chunked: trueto all outbound Roboflow API request headers by default.Changes
Content-Length validation removal
verify_content_lengthparameter fromget_from_url()and_get_from_url()ininference/core/roboflow_api.py, and deleted the Content-Length validation block.verify_content_length=Trueto use the new signatures:get_roboflow_model_data,get_roboflow_instant_model_datainroboflow_api.pyget_from_urlininference/core/models/roboflow.py(environment and model weights)get_from_urlininference/models/sam3/visual_segmentation.pyandinference/models/sam3/segment_anything3.pyContent-Lengthfrom response mocks intest_get_roboflow_model_data_when_response_parsing_error_occursandtest_get_roboflow_model_data_when_valid_response_expected.MD5 verification warning (no
x-goog-hash)MD5_VERIFICATION_ENABLEDisTrueand the response has nox-goog-hashheader, a warning is logged including the request URL.MD5_VERIFICATION_ENABLEDisTrueand there is a response that hasx-goog-hashheader, but it's md5 part is missing, warning is logged including the request URL.scheme,netlocandpath.test_get_from_url_when_md5_verification_enabled_but_x_goog_hash_header_missing– warning is logged and the request still succeeds.test_get_from_url_when_md5_verification_enabled_but_x_goog_hash_missing_does_not_log_api_key– URL containsapi_keybut the logged message does not contain the secret.New request header:
X-Allow-ChunkedALLOW_CHUNKED_RESPONSE_HEADER = "X-Allow-Chunked"and set it to"true"inbuild_roboflow_api_headers(), so every outbound Roboflow API request (GET and POST) sends this header by default.test_build_roboflow_api_headers_*tests and theget_roboflow_workspace_asyncheader assertion were updated to expect this header.Testing
pytest tests/inference/unit_tests/core/test_roboflow_api.py— all relevant tests updated or added; no new failures.