Skip to content
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

SafeBrowsing v5 only supports alt=proto #2534

Closed
lionello opened this issue Apr 19, 2024 · 5 comments
Closed

SafeBrowsing v5 only supports alt=proto #2534

lionello opened this issue Apr 19, 2024 · 5 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@lionello
Copy link

lionello commented Apr 19, 2024

This should be "default": "proto":

The currently generated SDK will always fail with HTTP 400, response:

{
  "error": {
    "code": 400,
    "message": "Unsupported Output Format",
    "status": "INVALID_ARGUMENT"
  }
}

Setting alt to "proto" doesn't work, since the SDK always decodes the response as JSON, failing with invalid character '\x12' looking for beginning of value trying to decode the protobuf as JSON.

@lionello lionello changed the title SafeBrowsing v5 only support alt=proto SafeBrowsing v5 only supports alt=proto Apr 19, 2024
@codyoss codyoss assigned quartzmo and unassigned codyoss Apr 19, 2024
@quartzmo
Copy link
Member

APIs that publish clients to this repo should be sending JSON responses. If the SafeBrowsing v5 API is sending protobuf responses, that is incorrect. Can you provide a full dump of the response?

@lionello
Copy link
Author

lionello commented Apr 19, 2024

That JSON was the full dump of the response. I can capture the HTTP headers later.
When alt=json you always get that error JSON. The SafeBrowsing v5 API only handles requests when alt=proto.

@quartzmo
Copy link
Member

Here is the discovery document for this API: https://safebrowsing.googleapis.com/$discovery/rest?version=v5

We will investigate further.

@quartzmo quartzmo added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 23, 2024
@quartzmo
Copy link
Member

quartzmo commented Apr 24, 2024

I believe the discovery document for this API (linked above) states that JSON is supported and is the default:

    "alt": {
      "enumDescriptions": [
        "Responses with Content-Type of application/json",
        "Media download with context-dependent Content-Type",
        "Responses with Content-Type of application/x-protobuf"
      ],
      "default": "json",
      "type": "string",
      "enum": [
        "json",
        "media",
        "proto"
      ],
      "location": "query",
      "description": "Data format for response."
    },

Therefore it appears that the service does not honor the contract of the discovery document.

@lionello Can you search the Cloud SQL issue tracker for an existing issue, and if not found, create a new issue demonstrating how the service does not support alt=json, then link that issue from here?

@codyoss
Copy link
Member

codyoss commented Apr 30, 2024

Closing. As stated above this issue should be reported to the API service team.

@codyoss codyoss closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants