Skip to content

martian: support server sent events without chunked encoding#812

Merged
mmatczuk merged 3 commits intomainfrom
hg/fix_sse
May 28, 2024
Merged

martian: support server sent events without chunked encoding#812
mmatczuk merged 3 commits intomainfrom
hg/fix_sse

Conversation

@Choraden
Copy link
Contributor

@Choraden Choraden commented May 23, 2024

This patch applies patternFlushWriter to support

  • non-chunked SSE i.e. flush after '\n\n'
  • chunked-encoding i.e. flush after '\r\n'

@Choraden Choraden changed the title martian martian: support server sent events without chunked encoding May 23, 2024
}

// flushAfterSSEWriter forces a flush after each '\n\n' is written.
type flushAfterSSEWriter struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need a flushWriter that can be used in proxy conn and handler for sse and chunked encoding.

type interface flusher Flush() error

type afterFlusher struct {
  w io.Writer
  f flusher
  eom [2]byte
}

This needs to come with a set of extensive tests.

This needs to be applied everywhere.

From http.Server:

HTTP/1.1 or greater: Transfer-Encoding has been set to identity, and no
content-length has been provided. The connection must be closed after the
reply is written, and no chunking is to be done. This is the setup
recommended in the Server-Sent Events candidate recommendation 11,
section 8.
if hasTE && te == "identity" {
	cw.chunking = false
	w.closeAfterReply = true
	delHeader("Transfer-Encoding")
} else {
	// HTTP/1.1 or greater: use chunked transfer encoding
	// to avoid closing the connection at EOF.
	cw.chunking = true
	setHeader.transferEncoding = "chunked"
	if hasTE && te == "chunked" {
		// We will send the chunked Transfer-Encoding header later.
		delHeader("Transfer-Encoding")
	}
}
}

p = p[:n]
l := len(p)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it the same as n

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course!

} else {
switch {
case isTextEventStream(res):
err = res.Write(&patternFlushWriter{w: p.brw.Writer, f: p.brw.Writer, pattern: [2]byte{'\n', '\n'}})
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it makes sense to specify the special byte sequences as global variables.

} else {
switch {
case isTextEventStream(res):
err = res.Write(&patternFlushWriter{w: p.brw.Writer, f: p.brw.Writer, pattern: [2]byte{'\n', '\n'}})
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to have constructor.

w io.Writer
f flusher
pattern [2]byte
last byte
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add space before last?

Choraden added 2 commits May 27, 2024 13:50
patternFlushWriter will flush when specified pattern occurs.
This patch applies patternFlushWriter to support
- non-chunked SSE i.e. flush after '\n\n'
- chunked-encoding i.e. flush after '\r\n'
Copy link
Contributor

@mmatczuk mmatczuk left a comment

Choose a reason for hiding this comment

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

LGTM

@mmatczuk mmatczuk merged commit fe4243e into main May 28, 2024
@mmatczuk mmatczuk deleted the hg/fix_sse branch May 28, 2024 06:56
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.

2 participants