-
Notifications
You must be signed in to change notification settings - Fork 336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update: Give clearer message when header encoding fails #1088
base: main
Are you sure you want to change the base?
Conversation
"Only latin-1 encoding supported by HTTP RFC 2616, check headers and values for unusual chars", | ||
exc_info=uee, | ||
) | ||
raise BadGeneratorException from uee |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having read the reported issue I think the raise here may not be an appropriate type. BadGeneratorException
extends PluginConfigurationError
and this error is not related to the configuration at this time.
This error seems like a new exception class may be appropriate, something like BadResponseException
/ InvalidResponseError
/ MalformedResponseException
. The message helps the user understand that the issue is with the response data, however I am looking for something programatic
to make it clear that the target systems response is not supported vs suggesting that configuration of the generator could be changed to avoid the issue.
This catches an exception during request composition
…On Thu, Jan 30, 2025, 16:55 Jeffrey Martin ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In garak/generators/rest.py
<#1088 (comment)>:
> @@ -212,7 +212,16 @@ def _call_model(
"proxies": self.proxies,
"verify": self.verify_ssl,
}
- resp = self.http_function(self.uri, **req_kArgs)
+ try:
+ resp = self.http_function(self.uri, **req_kArgs)
+ except UnicodeEncodeError as uee:
+ # only RFC2616 (latin-1) is guaranteed
+ # don't print a repr, this might leak api keys
+ logging.error(
+ "Only latin-1 encoding supported by HTTP RFC 2616, check headers and values for unusual chars",
+ exc_info=uee,
+ )
+ raise BadGeneratorException from uee
Having read the reported issue I think the raise here may not be an
appropriate type. BadGeneratorException extends PluginConfigurationError
and this error is not related to the configuration at this time.
This error seems like a new exception class may be appropriate, something
like BadResponseException / InvalidResponseError /
MalformedResponseException. The message helps the user understand that
the issue is with the response data, however I am looking for something
programatic to make it clear that the target systems response is not
supported vs suggesting that configuration of the generator could be
changed to avoid the issue.
—
Reply to this email directly, view it on GitHub
<#1088 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5YTVXEK5UDNK4FGWK4UL2NJDQXAVCNFSM6AAAAABV5NRIZ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBUGMZDAOBWGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
resolves #1035 (best effort)
HTTP under base RFC2616 only supports ASCII and latin-1 (iso-8859-1) encodings. This is echoed in Python's
http
lib implementation. When characters from outside these sets turn up in certain parts of an HTTP requests, an encoding exception is raised.This PR catches that exception, wraps it in an error message, logs with a suggestion, and then tries to stop generation using a BadGeneratorException.
Design decisions:
Verification
python -m pytest tests/generators/test_rest::test_rest_non_latin1