JSON targets format #300

tsenart · 2018-06-29T20:23:03Z

Background

The original targets format closely resembles plain HTTP/1.x messages. It was meant to be used by humans writing those targets directly on their terminals. With its lack of support for in-line body definitions, this ill-defined legacy format made Vegeta hard to integrate with other programs generating dynamic targets on the fly with distinct request bodies.

Extending the legacy format to support arbitrary in-line request bodies would require further deviation from the HTTP/1.x message format since, to the best of my knowledge, there is no official plain text framing format that would allow the boundaries between multiple targets with arbitrary request bodies to be easily recognized (and hence, parsed).

It could be done, certainly, but for the use case of integrating Vegeta with other programs generating targets, I think a simpler alternative JSON based format could be better.

This PR proposes just such a format and is intended to gather feedback from interested parties.

It's currently a work-in-progress, but the CLI interface is already specified and documented.

Please provide feedback: @coxx, @1u0, @mbrookes1304, @cwinters, @xla.

Related Issues and PRs

Checklist

Git commit messages conform to community standards.
Each Git commit represents meaningful milestones or atomic units of work.
Changed or added code is covered by appropriate tests.

This commit introduces the following changes in preparation for alternative target formats: - LazyTargeter renamed to LegacyTargeter - EagerTargeter refactored to take in any Targeter - `-format` flag added to the attack command - Extended documentation of the legacy target format

golangci · 2018-06-29T20:23:17Z

attack.go

-		return err
+
+	switch opts.format {
+	case "json":


string json has 3 occurrences, make it a constant

Agree, should be constants.

OK, I'll make some constants for these values.

matthewbrookes · 2018-07-02T07:38:16Z

@tsenart I like the approach taken here. I think it's much cleaner creating a new format than trying to find an ugly way to fit this feature into the legacy format

xla

This proposal is sound to the bone. The only gripe I have is with the naming of the formats. While json communicates the expected input format, legacy implies lifecycle. There are certainly perceived limitations for the current format, yet it might serve well for a good amount of use-cases. So I'd propose to give it a name which aligns well e.g.: plain, text or something similar.

xla · 2018-07-04T09:51:07Z

README.md

+
+##### `legacy` format
+
+The ill-defined legacy format almost resembles the plain-text HTTP message format but


In here ill-defined seems self-deprecating, could be left out.

OK, agree there's no need to deprecate it. I'll change the wording.

xla · 2018-07-04T09:51:47Z

attack.go

@@ -25,6 +25,7 @@ func attackCmd() command {

 	fs.StringVar(&opts.name, "name", "", "Attack name")
 	fs.StringVar(&opts.targetsf, "targets", "stdin", "Targets file")
+	fs.StringVar(&opts.format, "format", "legacy", "Targets format [legacy, json]")


Defaults could be constants.

They could. I'll make this change together with the other constant related changes.

xla · 2018-07-04T09:52:17Z

attack.go

-		return err
+
+	switch opts.format {
+	case "json":


Agree, should be constants.

xla · 2018-07-04T09:53:49Z

attack.go

+		tr = vegeta.NewJSONTargeter(src, body, hdr)
+	case "legacy":
+		fallthrough
+	default:


The user should get feedback for wrong inputs, would move this up to the case where it belongs and error for everything else.

Yep, this implementation was rather incomplete in view of getting earlier feedback. I'll add proper validation as well as tests.

xla · 2018-07-04T10:00:04Z

README.md

+
+The JSON format makes integration with programs that produce targets dynamically easier.
+Each target is one JSON object in its own line. If present, the body field must be base64 encoded.
+


This section should be expanded with the fileds of the json objects, which are mandatory and which are optional.

+1. What do you think of checking in a JSON schema file for this? Is it over-kill?

That's a great idea, especially with such small surface,maintaining such a file will be great for documentation and potential integrations.

xla · 2018-07-04T10:00:46Z

lib/targets.go

+	Method string      `json:"method"`
+	URL    string      `json:"url"`
+	Body   []byte      `json:"body"`
+	Header http.Header `json:"header"`


Always uncertain which one is canonical Header or Headers.

An HTTP message has a Header and a Body. A Header containers many "Headers". Yes, it's confusing. What naming do you think we should stick to?

Usually I consult the rfc and it's consistently referring to that part of request/response as header, so all good here. :>

tsenart · 2018-07-05T19:19:44Z

@xla: So, what do you prefer? text? I feel like it is too general... The format is "almost" like HTTP. Could we find something along those lines? Would calling it http be too much of a lie? What about httpish? 😺

matthewbrookes · 2018-07-02T07:34:29Z

README.md

-  -reporter string
-      Reporter [text, json, plot, hist[buckets]] (default "text")
-```
+### report command


Any particular reason why you have removed the explanation of report and dump command?

Because it was redundant. It's already included in the output of vegeta -h.

1u0 · 2018-07-05T22:14:54Z

Imo, the drawback of proposed api is that body payloads are not in plain text (base64 encoded).
If you remove base64, but still keep json format as "envelope", then for json body payloads one needs to escape them.
Both things reduce human readability of targets, text searching.

As an alternative, maybe yaml format can be used as an "envelope", this way allowing to put unescaped body payloads.

Or, you may re-consider the idea with HTTP/1.x message format. For example, the approach used in REST client tools (with inline body payloads and ### as requests separator):

IntelliJ IDEA;
Visual Studio Code plugin: https://github.com/Huachao/vscode-restclient

This would also make vegeta interop with those tools as a bonus.

tsenart · 2018-07-06T07:11:25Z

@1u0 Thanks for bringing that seemingly popular format to our attention! I wasn't aware of it personally. Do you know if it's more formally specified somewhere? What are its origins?

The only issue I can see is that framing scheme doesn't allow for request bodies to contain that same separator ### in them. That's one issue the Base64 encoding in the JSON format solves. How problematic do you think it could be in practice? I can imagine people writing programs to generate Vegeta targets dynamically with request body contents they don't know and hitting this potentially surprising issue.

Regarding the JSON format, it isn't clear from what you wrote why you find the need to Base64 encode the request bodies a drawback. Could you show us how it hinders you so that we understand your context better?

1u0 · 2018-07-06T10:03:28Z

Regarding the JSON format, it isn't clear from what you wrote why you find the need to Base64 encode the request bodies a drawback.

A potential use case can be that payload was generated (persisted into a file) and passed to vegeta. But later on, the payload needs to be manually analysed/checked. With plain text, one may just use common tools (like grep, less) to read and search the payload file.
With your proposal it should be also possible to achieve this, for example by adding jq to "unwrap" the payload. It's just a small inconvenience compared with "plain text" approach.

About the `.http` file format:

Do you know if it's more formally specified somewhere? What are its origins?

I'm not aware about any formal specification of it.
My guess that JetBrains came up with this format in their (proprietary) plugin, also inspired by HTTP message format (they introduced it in their blog posts). The plugin for VS Code, looks like, tries to follow the semantics as close as possible to one in IntelliJ IDEA.

The only issue I can see is that framing scheme doesn't allow for request bodies to contain that same separator ### in them.

True, but this is limited only at the start of new lines. There is also a potentially conflicting "< -rule" to include request body from external file. Both patterns can conflict with real payload.
Imo, it's a design tradeoff.

tsenart · 2018-07-06T11:09:13Z

A potential use case can be that payload was generated (persisted into a file) and passed to vegeta. But later on, the payload needs to be manually analysed/checked. With plain text, one may just use common tools (like grep, less) to read and search the payload file.
With your proposal it should be also possible to achieve this, for example by adding jq to "unwrap" the payload. It's just a small inconvenience compared with "plain text" approach.

You don't need jq to unwrap the payload. Most UNIX environments have a dedicated tool for that. In macOS, that's called base64. I understand your nuisance, but I think it's a minor one, and these tools are easy to use and compose in the terminal. Additionally, if you have a program generating targets, any programming language has a base64 module that's easy to use.

True, but this is limited only at the start of new lines. There is also a potentially conflicting "<-rule" to include request body from external file. Both patterns can conflict with real payload.
Imo, it's a design tradeoff.

Yeah, indeed a tradeoff. I suppose if it's well documented, then it shouldn't be a problem for people. So I'd be on-board with also adding this functionality to the HTTP-like format, but I'm wondering if breaking back-wards compatibility in this particular case is worth it. I'm not very sure. What's your take @xla?

xla · 2018-07-06T16:47:14Z

@xla: So, what do you prefer? text? I feel like it is too general... The format is "almost" like HTTP. Could we find something along those lines? Would calling it http be too much of a lie? What about httpish?

Ha! httpy maybe. 👹 I think http is fine as long as the caveats are well-documented.

xla · 2018-07-06T16:55:43Z

Yeah, indeed a tradeoff. I suppose if it's well documented, then it shouldn't be a problem for people. So I'd be on-board with also adding this functionality to the HTTP-like format, but I'm wondering if breaking back-wards compatibility in this particular case is worth it. I'm not very sure. What's your take @xla?

I'd focus on the proposed change for the json format at first and keep the additonal changes to the existing format as potential improvement for the backlog and evaluate in depth if the change is indeed breaking.

The disucssion around encoding of payload bodies seems moot as any sensible human interaction with formats like json needs specific tooling, as much as it is necessary to interact with large amounts of newline delimited plain text. It should not so much inform the design of the implementation as it seems reasonable to assume that the majoirty of interactions with the json definitions come from machine owned pipelines, basically supporting your claim @tsenart that json targets are generated for the most part.

tsenart · 2018-07-06T19:40:31Z

I'd focus on the proposed change for the json format at first and keep the additonal changes to the existing format as potential improvement for the backlog and evaluate in depth if the change is indeed breaking.

+1

golangci · 2018-07-07T14:30:10Z

internal/cmd/jsonschema/main.go

+	typ := fs.String("type", "", fmt.Sprintf("Vegeta type to generate a JSON schema for [%s]", valid))
+	out := fs.String("output", "stdout", "Output file")
+
+	fs.Parse(os.Args[1:])


Error return value of fs.Parse is not checked

tsenart · 2018-07-07T16:09:41Z

@xla: Assigned to you. PTAL again.

xla

👍 🍡

Minor style nits. :>

xla · 2018-07-10T07:29:18Z

attack.go

@@ -25,6 +26,8 @@ func attackCmd() command {

 	fs.StringVar(&opts.name, "name", "", "Attack name")
 	fs.StringVar(&opts.targetsf, "targets", "stdin", "Targets file")
+	fs.StringVar(&opts.format, "format", vegeta.HTTPTargetFormat,
+		fmt.Sprintf("Targets format [%s]", strings.Join(vegeta.TargetFormats, ", ")))


I find this breaking odd, it doesn't give consistent guidance how to break long lines and makes it hard to edit. Might be preferrable to use the following rule:

func myFunc( param1, param2, param3, )

Let's discuss this today in our pairing session.

xla · 2018-07-10T07:29:38Z

attack.go

+	case vegeta.HTTPTargetFormat:
+		tr = vegeta.NewHTTPTargeter(src, body, hdr)
+	default:
+		return fmt.Errorf("format %q isn't one of [%s]",


Same as above.

xla · 2018-07-10T07:33:05Z

lib/targets.go

+	case t == nil || other == nil:
+		return false
+	default:
+		equal := t.Method == other.Method &&


Guess this is for readability, would argue just as hard to read in the if statement itself.

tsenart · 2018-07-10T08:31:09Z

This feature has been looooooong in the making. Since #148, namely. Feels good to make progress again :-)

tsenart · 2018-07-10T08:31:33Z

By feature, I mean in-line response bodies which JSON targets make easy peasy.

omjego · 2020-04-23T15:03:26Z

Hey @tsenart , what's the status of this feature? Is it merged already?

tsenart · 2020-04-23T17:07:44Z

Yes. Sent via Superhuman iOS ( https://sprh.mn/?vip=tsenart@gmail.com )

…

On Thu, Apr 23 2020 at 5:03 PM, Omkar Jadhav < ***@***.*** > wrote: Hey @tsenart ( https://github.com/tsenart ) , what's the status of this feature? Is it merged already? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub ( #300 (comment) ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AAAQPDZ2PMHFBJHH55IYA6TROBKE7ANCNFSM4FHWJ74Q ).

tsenart added 2 commits June 29, 2018 19:25

attack: Implement JSON target format

50ef463

tsenart added the WIP label Jun 29, 2018

golangci reviewed Jun 29, 2018

View reviewed changes

xla suggested changes Jul 4, 2018

View reviewed changes

tsenart mentioned this pull request Jul 5, 2018

Refactor target parser #141

Closed

Merge branch 'master' into json-targets

909e0fe

tsenart mentioned this pull request Jul 5, 2018

Support targets with inline bodies similar to shell's "here tag" #260

Closed

matthewbrookes reviewed Jul 5, 2018

View reviewed changes

tsenart added 5 commits July 7, 2018 15:04

targets: Rename legacy format to http

a7df0cf

targets: Introduce constants for different formats

ec323fa

attack cmd: Return error on bad format selection

188262a

attack cmd: Re-use target format constants

dae3d2d

targets: Generate vegeta.Target JSON schema

eb227c2

golangci reviewed Jul 7, 2018

View reviewed changes

tsenart added 2 commits July 7, 2018 18:02

targets: Test and refine JSONTargeter

2e261e6

jsonschema: Exit on fs.Parse error

4911bc1

tsenart assigned tsenart and xla and unassigned tsenart Jul 7, 2018

targets: Refactor EagerTargeter to ReadAllTargets

6de2156

xla removed the WIP label Jul 10, 2018

xla approved these changes Jul 10, 2018

View reviewed changes

xla assigned tsenart and unassigned xla Jul 10, 2018

tsenart merged commit 966dd85 into master Jul 10, 2018

tsenart deleted the json-targets branch July 10, 2018 08:29


		##### `legacy` format

		The ill-defined legacy format almost resembles the plain-text HTTP message format but


		The JSON format makes integration with programs that produce targets dynamically easier.
		Each target is one JSON object in its own line. If present, the body field must be base64 encoded.

JSON targets format #300

JSON targets format #300

Conversation

tsenart commented Jun 29, 2018 • edited Loading

Background

Related Issues and PRs

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbrookes commented Jul 2, 2018

xla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsenart commented Jul 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1u0 commented Jul 5, 2018

tsenart commented Jul 6, 2018 • edited Loading

1u0 commented Jul 6, 2018

About the .http file format:

tsenart commented Jul 6, 2018

xla commented Jul 6, 2018

xla commented Jul 6, 2018 • edited Loading

tsenart commented Jul 6, 2018

Choose a reason for hiding this comment

tsenart commented Jul 7, 2018

xla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsenart commented Jul 10, 2018

tsenart commented Jul 10, 2018

omjego commented Apr 23, 2020

tsenart commented Apr 23, 2020 via email

tsenart commented Jun 29, 2018 •

edited

Loading

tsenart commented Jul 6, 2018 •

edited

Loading

About the `.http` file format:

xla commented Jul 6, 2018 •

edited

Loading