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

add http-check and http-modify filters #48089

Merged
merged 2 commits into from
Jan 23, 2025
Merged

add http-check and http-modify filters #48089

merged 2 commits into from
Jan 23, 2025

Conversation

jkarneges
Copy link
Member

@jkarneges jkarneges commented Dec 18, 2024

This adds two new message filters that allow delegating filtering logic to an HTTP server:

  • http-check: Sends metadata only (no message content) to the server, to determine the action (send or drop).
  • http-modify: Sends metadata and message content to the server, to determine the action and potentially have the content modified.

For both filters, the target endpoint is specified using the subscription metadata field url. Requests use the POST method and contain Sub-Meta, Pub-Meta, and Grip-Last headers. For the http-modify filter, the message content is included in the body (http-check filter leaves the body empty). The server is expected to reply with a status code 200 or 204, and optionally an Action: drop header to indicate a drop action. For http-modify, code 200 means the replacement content is in the response body, and code 204 means no change to the content.

@jkarneges jkarneges force-pushed the jkarneges/http-filter branch from 5fc4533 to d76c190 Compare January 7, 2025 20:19
@jkarneges jkarneges force-pushed the jkarneges/http-filter branch 2 times, most recently from e14e36f to 92f2a6d Compare January 15, 2025 22:17
@jkarneges jkarneges marked this pull request as ready for review January 15, 2025 22:20
@jkarneges jkarneges force-pushed the jkarneges/http-filter branch from 92f2a6d to fd8885f Compare January 17, 2025 23:08
@jkarneges jkarneges marked this pull request as draft January 17, 2025 23:11
@jkarneges jkarneges force-pushed the jkarneges/http-filter branch 2 times, most recently from a7886f9 to 9a8e000 Compare January 22, 2025 19:01
@jkarneges jkarneges marked this pull request as ready for review January 22, 2025 19:02
@jkarneges jkarneges force-pushed the jkarneges/http-filter branch from 9a8e000 to 19f56ac Compare January 22, 2025 19:09
Copy link

@mhp mhp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the context to fully understand what's going on here, but there was one bit that stood out as potentially odd. Hope that's helpful!

Comment on lines +390 to +394
while(it.hasNext())
{
it.next();
vmap[it.key()] = it.value();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like an odd construction. I don't have the context on exactly what you are doing here but it looks like you are skipping the first item in the iterator by calling it.next() before copying. Is that intentional? I guess it must be, but maybe a comment as to what you are skipping might help make this look a little less odd! Same with the two loops that follow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The iterator requires calling next() to access the first item, so the code is intentional and not skipping the first item. It's a quirk of Qt's unintuitive "Java-style" API:

Unlike STL-style iterators, Java-style iterators point between items rather than directly at items. The first call to next() advances the iterator to the position between the first and second item, and returns the first item; the second call to next() advances the iterator to the position between the second and third item; and so on.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh! What an odd interface! Well, that makes sense - thanks for explaining it.

@jkarneges jkarneges merged commit 5a9fc08 into main Jan 23, 2025
19 checks passed
@jkarneges jkarneges deleted the jkarneges/http-filter branch January 23, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants