Pipeline test file generation escapes special characters in golden files generation

[P1llus]

When creating new expected.json files from raw log files, it seems that certain characters are HTML escaped, adding in unicode characters in the generated output.

I would like to know is this is the intended results, as it differs from how the output looked like in beat modules test file generation (which used python).

An example would be zeek logs, here characters like <> is turned into \u003c and \u003e.

Which would turn this example raw event:

        "zeek.session_id": "CPRLCB4eWHdjP852Bk",
        "zeek.sip.call_id": "8694cd7e-976e4fc3-d76f6e38@172.16.133.19",
        "zeek.sip.request.body_length": 0,
        "zeek.sip.request.from": "\"AppNeta\" <sip:116954_Boston6@newyork.voip.ms>",
        "zeek.sip.request.path": [
            "SIP/2.0/UDP 172.16.133.19:5060"
        ],
        "zeek.sip.request.to": "<sip:116954_Boston6@newyork.voip.ms>",
        "zeek.sip.response.body_length": 0,
        "zeek.sip.response.from": "\"AppNeta\" <sip:116954_Boston6@newyork.voip.ms>",
        "zeek.sip.response.path": [
            "SIP/2.0/UDP 172.16.133.19:5060"
        ],
        "zeek.sip.response.to": "<sip:116954_Boston6@newyork.voip.ms>;tag=as023f66a5",
        "zeek.sip.sequence.method": "REGISTER",
        "zeek.sip.sequence.number": "4127",
        "zeek.sip.status.code": 401,
        "zeek.sip.status.msg": "Unauthorized",
        "zeek.sip.transaction_depth": 0,
        "zeek.sip.uri": "sip:newyork.voip.ms:5060",
        "zeek.sip.user_agent": "PolycomSoundStationIP-SSIP_5000-UA/3.2.4.0267"

To this:

"zeek": {
                "session_id": "CPRLCB4eWHdjP852Bk",
                "sip": {
                    "request": {
                        "path": [
                            "SIP/2.0/UDP 172.16.133.19:5060"
                        ],
                        "from": "\"AppNeta\" \u003csip:116954_Boston6@newyork.voip.ms\u003e",
                        "to": "\u003csip:116954_Boston6@newyork.voip.ms\u003e",
                        "body_length": 0
                    },
                    "sequence": {
                        "method": "REGISTER",
                        "number": "4127"
                    },
                    "response": {
                        "path": [
                            "SIP/2.0/UDP 172.16.133.19:5060"
                        ],
                        "from": "\"AppNeta\" \u003csip:116954_Boston6@newyork.voip.ms\u003e",
                        "to": "\u003csip:116954_Boston6@newyork.voip.ms\u003e;tag=as023f66a5",
                        "body_length": 0
                    },
                    "id": {},
                    "uri": "sip:newyork.voip.ms:5060",
                    "transaction_depth": 0,
                    "user_agent": "PolycomSoundStationIP-SSIP_5000-UA/3.2.4.0267",
                    "call_id": "8694cd7e-976e4fc3-d76f6e38@172.16.133.19",
                    "status": {
                        "msg": "Unauthorized",
                        "code": 401
                    }
                }
            },

[P1llus]

This could be resolved by disabling html escaping using:
https://golang.org/pkg/encoding/json/#Encoder.SetEscapeHTML

Which I guess would need to be set somewhere around here:

elastic-package/internal/testrunner/runners/pipeline/test_result.go

Line 143 in 003c019

func marshalTestResultDefinition(result *testResult) ([]byte, error) {

[mtojek]

Thank for opening this issue, @P1llus .

Is it because of readability, compatibility with Beats or do you see any other concern? It works this way as it's default behavior in Go standard library and I'm not sure if we want to change it (unless we have a good justification). As you mentioned both forms are correct :)

Are you working on migrating tests from Beats to integrations?

cc @ycombinator

[P1llus]

Thanks for the quick response @mtojek ! :)

This is really only because of readability, as long as it won't create any issues when the pipeline tester will try to compare then it's not really an issue for me, I just wanted to make sure it's intended behavior.

Just to mention that the actual output of a beat to ES is not escaped, so the golden files would differentiate from the real implementation that way, but not in a big way.

I am currently in the process of removing all the edge processing and moving them to pipelines, and at the same time I am also adding pipeline test files to all the packages that are missing them, starting with the ones on the list here: https://github.com/elastic/security-team/issues/791

[P1llus]

If its only a decision if we want to keep it this way or not, then I would like to ping @andrewkroh as well, for me its fine to keep it that way, but if all we need to do is disable the html encoding on the encoder, it's a pretty quick fix, but might need to regenerate some generated files to remove the html encoding from them.

[mtojek]

I don't have any preference here :) If it's more convenient to disable escaping, I would be for doing it. Let's collect more feedback here.

[ycombinator]

If its a simple change to disable HTML encoding (which it sounds like it is), I'm +1 for making it to improve readability.

[andrewkroh]

I think we should make the change. These odd representations are showing up in our docs.

elastic-package format also needs changed, otherwise it will undo the results of the pipeline test generator.

elastic-package/internal/formatter/json_formatter.go

Line 21 in 43c68bc

formatted, err := json.MarshalIndent(&rawMessage, "", " ")

[jsoriano]

We can maybe apply this to packages with the next package spec version, this way it should be a small change. I will include it in the current sprint.

[jsoriano]

Reopening, something seems to be still encoding HTML entities https://github.com/elastic/integrations/pull/8478/files#r1391779070