From 0b093ae6bbe42b09e993874b41c7670f292ca38a Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Tue, 25 Aug 2026 14:55:58 +0000 Subject: [PATCH] feat(article): render embedded images in terminal Fetch article images only for terminal presentation, while keeping raw Markdown output unchanged. Reuse the configured image renderer with bounded, resolver-aware SSRF protection and update the article documentation. --- docs/article.md | 8 +- docs/cli-reference.md | 4 +- docs/image-rendering.md | 4 +- docs/limits.md | 7 +- docs/output-formatting.md | 3 +- internal/client/client.go | 20 ++ internal/fetch/article_images.go | 261 +++++++++++++++++++++++++ internal/fetch/article_images_test.go | 47 +++++ internal/fetch/fetch.go | 34 +++- internal/fetch/format_response_test.go | 4 +- internal/fetch/retry.go | 2 +- internal/format/markdown.go | 35 +++- internal/format/markdown_test.go | 23 +++ internal/image/block.go | 13 +- internal/image/image.go | 20 +- internal/image/inline.go | 10 +- internal/image/kitty.go | 22 ++- 17 files changed, 470 insertions(+), 47 deletions(-) create mode 100644 internal/fetch/article_images.go create mode 100644 internal/fetch/article_images_test.go diff --git a/docs/article.md b/docs/article.md index 27381c3f..924fc4d0 100644 --- a/docs/article.md +++ b/docs/article.md @@ -45,8 +45,12 @@ Only available fields are emitted. String values use JSON string quoting, which is a safe YAML scalar. `length` is numeric. Markdown pass-through emits only `url`. -Terminal output may use the normal formatter, color, and pager. Output files, -pipes, and clipboard destinations receive raw, uncolored Markdown. +Terminal output may use the normal formatter and color. On a terminal, +article images are fetched and rendered with the configured image policy. Use +`--image off` to disable this behavior. Article images are not fetched for +output files, pipes, or clipboard destinations, which receive raw, uncolored +Markdown. Image fetches are bounded and failed images fall back to their alt +text and URL. Article mode cannot be combined with WebSockets, gRPC, DNS/TLS inspection, `--discard`, `--remote-name`, or `--remote-header-name`. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 9d5fcaef..9be3293c 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -257,7 +257,9 @@ fetch --format on example.com # Force formatting Extract a readable HTML/XHTML page as Markdown with YAML frontmatter. Markdown responses (`text/markdown` and `text/x-markdown`) pass through after a `url` frontmatter field. Article mode buffers at most 16 MiB of decoded content and -does not execute JavaScript. It cannot be combined with `--discard`, +does not execute JavaScript. On terminals, embedded article images are fetched +and rendered unless `--image off` is set. Output files and pipes retain image +links in the raw Markdown. It cannot be combined with `--discard`, `--remote-name`, or `--remote-header-name`. The frontmatter fields are `title`, `byline`, `site_name`, `published_time`, diff --git a/docs/image-rendering.md b/docs/image-rendering.md index f2710959..abc8af1e 100644 --- a/docs/image-rendering.md +++ b/docs/image-rendering.md @@ -15,7 +15,9 @@ Control how images are rendered: | `off` | Disable image rendering | `native` remains accepted as a compatibility alias for `auto`. External -programs never run in `auto` mode. +programs never run in `auto` mode. When `--article` formats a document on a +terminal, embedded images use this same policy. Images are not fetched for +files, pipes, or clipboard output. ```sh fetch --image auto example.com/photo.jpg diff --git a/docs/limits.md b/docs/limits.md index 1d3a952b..9dc423c0 100644 --- a/docs/limits.md +++ b/docs/limits.md @@ -53,7 +53,8 @@ contain `key`, `token`, `secret`, `password`, `credential`, `signature`, or visible. HAR artifacts are the exception because they are intended to reproduce the exchange and can contain sensitive data. -Image rendering also rejects decoded dimensions above 8192 by 8192. External -image adapters and pagers run without a shell, with bounded output and a -process deadline. See [output formatting](output-formatting.md), [HAR](har.md), +Image rendering also rejects decoded dimensions above 8192 by 8192. Article +image fetching allows at most 16 images, 8 MiB per image, and 32 MiB in total. +External image adapters and pagers run without a shell, with bounded output and +a process deadline. See [output formatting](output-formatting.md), [HAR](har.md), [updates](updates.md), and [Agent Skill](agent-skill.md) for feature details. diff --git a/docs/output-formatting.md b/docs/output-formatting.md index 1fd68742..9bb59483 100644 --- a/docs/output-formatting.md +++ b/docs/output-formatting.md @@ -51,7 +51,8 @@ URL after redirects to resolve links. Article mode decodes the response before extraction and accepts at most 16 MiB of decoded content. It does not run JavaScript, so content rendered only by client-side scripts is not available. Output files and pipes receive raw, -uncolored Markdown. On a terminal, `--format`, `--color`, and `--pager` affect +uncolored Markdown. On a terminal, embedded article images are fetched and +rendered unless `--image off` is set. `--format`, `--color`, and `--pager` affect presentation only. ```sh diff --git a/internal/client/client.go b/internal/client/client.go index 757cecc1..13d5d26f 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -871,6 +871,26 @@ func (c *Client) HTTPClient() *http.Client { return c.c } +// LookupIPAddr resolves host using the resolver configured for this client. +// It keeps policy checks consistent with the addresses used by its transport. +func (c *Client) LookupIPAddr(ctx context.Context, host string) ([]net.IPAddr, error) { + if c != nil && c.resolver != nil { + return c.resolver.LookupIPAddr(ctx, host) + } + return net.DefaultResolver.LookupIPAddr(ctx, host) +} + +// UsesProxy reports whether requests for target use a configured proxy. +// Callers that inspect the peer connection must account for the proxy because +// the peer is then the proxy, not the requested origin. +func (c *Client) UsesProxy(target *url.URL) bool { + if c == nil || target == nil { + return false + } + proxy, err := ProxyForURL(c.proxy, target) + return err == nil && proxy != nil +} + // ResolverProvenance identifies the resolver policy used by this client. It // is safe for diagnostics because resolver endpoints redact credentials during // parsing and display construction. diff --git a/internal/fetch/article_images.go b/internal/fetch/article_images.go new file mode 100644 index 00000000..57e66fb3 --- /dev/null +++ b/internal/fetch/article_images.go @@ -0,0 +1,261 @@ +package fetch + +import ( + "context" + "errors" + "io" + "net" + "net/http/httptrace" + "net/url" + "strings" + "time" + + "github.com/ryanfowler/fetch/internal/client" + "github.com/ryanfowler/fetch/internal/core" + imageoutput "github.com/ryanfowler/fetch/internal/image" +) + +const ( + maxArticleImages = 16 + maxArticleImageBytes = 8 << 20 + maxArticleImageTotal = 32 << 20 + articleImageTimeout = 15 * time.Second +) + +// articleImageFetcher resolves and renders Markdown images for an article's +// terminal presentation. It deliberately does not modify the Markdown source. +type terminalPresentationReader struct { + io.Reader +} + +func (terminalPresentationReader) terminalPresentation() {} + +type articleImageFetcher struct { + ctx context.Context + client *client.Client + mode core.ImageSetting + baseURL *url.URL + + images map[string][]byte + failed map[string]bool + count int + total int64 + rendered int +} + +func newArticleImageFetcher(ctx context.Context, c *client.Client, mode core.ImageSetting, pageURL string) *articleImageFetcher { + baseURL, err := url.Parse(pageURL) + if err != nil || baseURL == nil { + return nil + } + return &articleImageFetcher{ + ctx: ctx, + client: c, + mode: mode, + baseURL: baseURL, + images: make(map[string][]byte), + failed: make(map[string]bool), + } +} + +func (f *articleImageFetcher) render(destination string, dst io.Writer) bool { + if f == nil || f.client == nil || dst == nil || f.rendered >= maxArticleImages { + return false + } + target, ok := f.resolve(destination) + if !ok { + return false + } + key := target.String() + data, cached := f.images[key] + if !cached { + if f.failed[key] || f.count >= maxArticleImages || f.total >= maxArticleImageTotal { + return false + } + f.count++ + var err error + data, err = f.fetch(target) + if err != nil { + f.failed[key] = true + return false + } + if int64(len(data)) > maxArticleImageTotal-f.total { + f.failed[key] = true + return false + } + f.images[key] = data + f.total += int64(len(data)) + } + + if err := imageoutput.RenderWithModeTo(f.ctx, data, f.mode, dst); err != nil { + return false + } + f.rendered++ + return true +} + +func (f *articleImageFetcher) fetch(target *url.URL) ([]byte, error) { + ctx, cancel := context.WithTimeout(f.ctx, articleImageTimeout) + defer cancel() + + req, err := f.client.NewRequest(ctx, client.RequestConfig{URL: target}) + if err != nil { + return nil, err + } + // Do not forward the article request's custom headers or credentials to + // image hosts. The shared client still supplies the configured transport, + // proxy, TLS, compression, and cookie policy. + req.Header.Set("Accept", "image/*") + ctx = client.WithRedirectValidator(ctx, func(hop client.RedirectHop) error { + if hop.NextRequest == nil || !safeArticleImageURL(ctx, hop.NextRequest.URL, f.client.LookupIPAddr) { + return errors.New("article image redirect targets a private or non-public address") + } + return nil + }) + req = req.WithContext(ctx) + blockedPeer := false + if !f.client.UsesProxy(req.URL) { + req = req.WithContext(httptrace.WithClientTrace(req.Context(), &httptrace.ClientTrace{ + GotConn: func(info httptrace.GotConnInfo) { + if articleImagePrivatePeer(info.Conn) { + blockedPeer = true + _ = info.Conn.Close() + } + }, + })) + } + resp, err := f.client.Do(req) + if blockedPeer { + if resp != nil { + _ = resp.Body.Close() + } + return nil, errors.New("article image connection targets a private address") + } + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, io.ErrUnexpectedEOF + } + if resp.Request == nil || !safeArticleImageURL(ctx, resp.Request.URL, f.client.LookupIPAddr) { + return nil, io.ErrUnexpectedEOF + } + if resp.ContentLength > maxArticleImageBytes { + return nil, io.ErrShortBuffer + } + data, err := io.ReadAll(io.LimitReader(resp.Body, maxArticleImageBytes+1)) + if err != nil { + return nil, err + } + if len(data) > maxArticleImageBytes { + return nil, io.ErrShortBuffer + } + return data, nil +} + +func (f *articleImageFetcher) resolve(destination string) (*url.URL, bool) { + destination = strings.TrimSpace(destination) + if destination == "" || f.baseURL == nil { + return nil, false + } + target, err := url.Parse(destination) + if err != nil { + return nil, false + } + target = f.baseURL.ResolveReference(target) + target.Fragment = "" + if !safeArticleImageURL(f.ctx, target, f.client.LookupIPAddr) { + return nil, false + } + return target, true +} + +func safeArticleImageURL(ctx context.Context, target *url.URL, lookup func(context.Context, string) ([]net.IPAddr, error)) bool { + if target == nil || target.User != nil || target.Host == "" { + return false + } + switch strings.ToLower(target.Scheme) { + case "http", "https": + default: + return false + } + + host := strings.ToLower(strings.TrimSuffix(target.Hostname(), ".")) + if host == "localhost" || strings.HasSuffix(host, ".localhost") || strings.HasSuffix(host, ".local") { + return false + } + if ip := net.ParseIP(host); ip != nil { + return safeArticleImageIP(ip) + } + if ctx == nil { + ctx = context.Background() + } + if lookup == nil { + lookup = net.DefaultResolver.LookupIPAddr + } + addresses, err := lookup(ctx, host) + if err != nil || len(addresses) == 0 { + return false + } + for _, address := range addresses { + if !safeArticleImageIP(address.IP) { + return false + } + } + return true +} + +func safeArticleImageIP(ip net.IP) bool { + if ip == nil || ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || + ip.IsUnspecified() || ip.IsMulticast() { + return false + } + if v4 := ip.To4(); v4 != nil { + return !nonPublicArticleIPv4(v4) + } + // Documentation, benchmarking, discard, and reserved IPv6 ranges are not + // routable article destinations and must not be used to bypass this policy. + return !(len(ip) == net.IPv6len && + (ip[0] == 0x20 && ip[1] == 0x01 && ip[2] == 0x0d && ip[3] == 0xb8 || + ip[0] == 0x20 && ip[1] == 0x01 && ip[2] == 0x00 && ip[3] == 0x02 || + ip[0] == 0x3f && ip[1]&0xf0 == 0xf0 || + ip[0] == 0x00 && ip[1] == 0x01 && ip[2] == 0x00 && ip[3] == 0x00 && + ip[4] == 0x00 && ip[5] == 0x00 && ip[6] == 0x00 && ip[7] == 0x00)) +} + +func nonPublicArticleIPv4(ip net.IP) bool { + if len(ip) < net.IPv4len { + return true + } + switch { + case ip[0] == 0 || ip[0] == 127 || ip[0] >= 224: + return true + case ip[0] == 100 && ip[1] >= 64 && ip[1] <= 127: + return true // Shared address space, RFC 6598. + case ip[0] == 169 && ip[1] == 254: + return true + case ip[0] == 192 && (ip[1] == 0 || ip[1] == 2 || ip[1] == 88 || ip[1] == 168): + return true + case ip[0] == 198 && (ip[1] == 18 || ip[1] == 19 || ip[1] == 51): + return true + case ip[0] == 203 && ip[1] == 0 && ip[2] == 113: + return true + case ip[0] >= 240: + return true + default: + return false + } +} + +func articleImagePrivatePeer(conn net.Conn) bool { + if conn == nil { + return false + } + host, _, err := net.SplitHostPort(conn.RemoteAddr().String()) + if err != nil { + return false + } + ip := net.ParseIP(host) + return ip != nil && !safeArticleImageIP(ip) +} diff --git a/internal/fetch/article_images_test.go b/internal/fetch/article_images_test.go new file mode 100644 index 00000000..d6889729 --- /dev/null +++ b/internal/fetch/article_images_test.go @@ -0,0 +1,47 @@ +package fetch + +import ( + "context" + "net" + "net/url" + "testing" +) + +func TestSafeArticleImageURLRejectsNonPublicResolvedAddresses(t *testing.T) { + tests := []struct { + name string + ip string + }{ + {name: "shared", ip: "100.100.100.200"}, + {name: "metadata", ip: "169.254.169.254"}, + {name: "documentation", ip: "192.0.2.10"}, + {name: "ipv6 documentation", ip: "2001:db8::10"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + target, err := url.Parse("https://images.example/photo.jpg") + if err != nil { + t.Fatal(err) + } + lookup := func(context.Context, string) ([]net.IPAddr, error) { + return []net.IPAddr{{IP: net.ParseIP(test.ip)}}, nil + } + if safeArticleImageURL(context.Background(), target, lookup) { + t.Fatalf("safeArticleImageURL accepted %s", test.ip) + } + }) + } +} + +func TestSafeArticleImageURLAcceptsPublicResolvedAddress(t *testing.T) { + target, err := url.Parse("https://images.example/photo.jpg") + if err != nil { + t.Fatal(err) + } + lookup := func(context.Context, string) ([]net.IPAddr, error) { + return []net.IPAddr{{IP: net.ParseIP("203.0.114.10")}}, nil + } + if !safeArticleImageURL(context.Background(), target, lookup) { + t.Fatal("safeArticleImageURL rejected a public address") + } +} diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index 7fc7b030..c65aacec 100644 --- a/internal/fetch/fetch.go +++ b/internal/fetch/fetch.go @@ -489,7 +489,7 @@ func signAWSRequest(r *Request, req *http.Request) error { return nil } -func processResponse(ctx context.Context, r *Request, resp *http.Response, hadRedirects, hadRetries bool, metrics *connectionMetrics) (exitCode int, retErr error) { +func processResponse(ctx context.Context, r *Request, c *client.Client, resp *http.Response, hadRedirects, hadRetries bool, metrics *connectionMetrics) (exitCode int, retErr error) { if !r.IgnoreStatus { exitCode = getExitCodeForStatus(resp.StatusCode) } @@ -608,7 +608,7 @@ func processResponse(ctx context.Context, r *Request, resp *http.Response, hadRe // If --copy is requested, wrap the response body to capture raw bytes. cc := newClipboardCopier(r, resp) - body, err := formatResponse(ctx, r, resp, cc) + body, err := formatResponse(ctx, r, resp, cc, c) if err != nil { return 0, contextCauseOr(err, ctx) } @@ -664,14 +664,14 @@ func formatWithBoundedOutput(p *core.Printer, subsystem string, fn func(*core.Pr return out.Bytes(), nil } -func formatResponse(ctx context.Context, r *Request, resp *http.Response, cc *clipboardCopier) (io.Reader, error) { +func formatResponse(ctx context.Context, r *Request, resp *http.Response, cc *clipboardCopier, c *client.Client) (io.Reader, error) { // Avoid trying to format the response for HEAD requests. if resp.Request != nil && resp.Request.Method == "HEAD" { return nil, nil } if r.Article { - return formatArticleResponse(r, resp, cc) + return formatArticleResponse(ctx, r, resp, cc, c) } output, outputWarning, err := getOutputValueDetails(r, resp) @@ -839,7 +839,7 @@ func sameDestinationPath(first, second string) bool { return strings.EqualFold(first, second) && os.PathSeparator == '\\' } -func formatArticleResponse(r *Request, resp *http.Response, cc *clipboardCopier) (io.Reader, error) { +func formatArticleResponse(ctx context.Context, r *Request, resp *http.Response, cc *clipboardCopier, c *client.Client) (io.Reader, error) { _, charset := format.GetContentType(resp.Header) original := resp.Body decoded, err := article.ReadLimited(transcodeReader(original, charset)) @@ -891,9 +891,26 @@ func formatArticleResponse(r *Request, resp *http.Response, cc *clipboardCopier) } p := r.PrinterHandle.Stdout() - if err := format.FormatMarkdown(markdown, p); err != nil { + options := format.MarkdownOptions{} + var images *articleImageFetcher + if r.Image != core.ImageOff && p.IsTerminal() && c != nil && r.Output == "" && r.Format != core.FormatOff { + images = newArticleImageFetcher(ctx, c, r.Image, pageURL) + if images != nil { + options.RenderImage = func(destination string) bool { + return images.render(destination, p) + } + } + } + if err := format.FormatMarkdownWithOptions(markdown, p, options); err != nil { return nil, err } + if images != nil && images.rendered > 0 { + // Image protocols must not pass through a pager or the text binary + // guard. The reader marker below preserves the terminal-only output + // contract while keeping raw article output unchanged. + r.NoPager = true + return terminalPresentationReader{Reader: bytes.NewReader(p.Bytes())}, nil + } return bytes.NewReader(p.Bytes()), nil } @@ -916,7 +933,8 @@ func streamToStdout(r io.Reader, p *core.Printer, forceOutput, noPager, drainSup } func streamToStdoutWithPagerContent(ctx context.Context, r io.Reader, p *core.Printer, forceOutput, noPager, drainSuppressedBinary, silent bool, pagerMode core.PagerMode, contentType string) error { - if noPager || forceOutput || isImageContentType(contentType) { + _, terminalPresentation := r.(interface{ terminalPresentation() }) + if noPager || forceOutput || terminalPresentation || isImageContentType(contentType) { // Raw output must remain byte-oriented, and image protocol bytes must // never be fed to a text pager. This also makes --output - an explicit // pager bypass, matching its raw-output contract. @@ -935,7 +953,7 @@ func streamToStdoutWithPagerContent(ctx context.Context, r io.Reader, p *core.Pr // A terminal must not receive a response chunk until that chunk has // passed the binary classifier. The guard continues checking later chunks, // so a binary response cannot hide behind an initial text prefix. - if core.IsStdoutTerm && !forceOutput { + if core.IsStdoutTerm && !forceOutput && !terminalPresentation { guard := newBinaryGuardReader(r, drainSuppressedBinary, nil) stopClosing := closeReaderOnContext(ctx, guard) first := make([]byte, 64*1024) diff --git a/internal/fetch/format_response_test.go b/internal/fetch/format_response_test.go index 66ff53cf..72e61631 100644 --- a/internal/fetch/format_response_test.go +++ b/internal/fetch/format_response_test.go @@ -55,7 +55,7 @@ func TestFormatResponseStreamsNDJSONThroughReader(t *testing.T) { PrinterHandle: core.NewHandle(core.ColorOff), } - reader, err := formatResponse(context.Background(), r, resp, nil) + reader, err := formatResponse(context.Background(), r, resp, nil, nil) if err != nil { t.Fatalf("formatResponse returned error: %v", err) } @@ -96,7 +96,7 @@ func readFormattedResponse(t *testing.T, body []byte) []byte { PrinterHandle: core.NewHandle(core.ColorOff), } - reader, err := formatResponse(context.Background(), r, resp, nil) + reader, err := formatResponse(context.Background(), r, resp, nil, nil) if err != nil { t.Fatalf("formatResponse returned error: %v", err) } diff --git a/internal/fetch/retry.go b/internal/fetch/retry.go index eac58d88..e002eaf7 100644 --- a/internal/fetch/retry.go +++ b/internal/fetch/retry.go @@ -170,7 +170,7 @@ func retryableRequest(ctx context.Context, r *Request, c *client.Client, req *ht return 0, errors.New("request completed without a response") } defer func() { _ = resp.Body.Close() }() - return processResponse(requestCtx, r, resp, hadRedirects, attempt > 0, metrics) + return processResponse(requestCtx, r, c, resp, hadRedirects, attempt > 0, metrics) } if resp != nil { diff --git a/internal/format/markdown.go b/internal/format/markdown.go index f59dddad..1e8f5417 100644 --- a/internal/format/markdown.go +++ b/internal/format/markdown.go @@ -31,6 +31,11 @@ const ( // of this value. type MarkdownOptions struct { MaxWidth int + + // RenderImage renders a Markdown image for terminal presentation and + // returns true when it wrote the image. It is ignored for non-terminal + // output. The callback owns image fetching and safety policy. + RenderImage func(destination string) bool } // FormatMarkdown formats the provided Markdown to the Printer. @@ -69,7 +74,13 @@ func FormatMarkdownWithOptions(buf []byte, p *core.Printer, options MarkdownOpti width = markdownWidth(options.MaxWidth, core.GetTerminalCols()) } - r := &mdRenderer{printer: p, source: rest, width: width} + r := &mdRenderer{ + printer: p, + source: rest, + width: width, + imageRenderer: options.RenderImage, + renderedImages: make(map[*ast.Image]bool), + } r.tty = p.IsTerminal() return ast.Walk(doc, r.walk) } @@ -192,6 +203,8 @@ type mdRenderer struct { lineWidth int pendingSpace string tty bool + imageRenderer func(destination string) bool + renderedImages map[*ast.Image]bool } type mdTableCell struct { @@ -761,8 +774,28 @@ func (r *mdRenderer) walk(n ast.Node, entering bool) (ast.WalkStatus, error) { } case *ast.Image: + if !entering && r.renderedImages[v] { + delete(r.renderedImages, v) + return ast.WalkSkipChildren, nil + } if entering { target := markdownLinkDestination(v.Destination) + if r.tty && r.imageRenderer != nil { + r.flushPendingSpace() + if r.lineWidth > 0 { + r.writeLineBreak() + } + if r.imageRenderer(target) { + r.popAllAndRestore() + r.renderedImages[v] = true + // Image protocols emit their own terminal line. Keep the + // following Markdown content on a fresh logical line and do + // not render the image's alt text a second time. + r.pendingSpace = "" + r.lineWidth = 0 + return ast.WalkSkipChildren, nil + } + } r.flushPendingSpace() active := r.printer.StartHyperlink(target) r.links = append(r.links, active) diff --git a/internal/format/markdown_test.go b/internal/format/markdown_test.go index 928927d0..e3511642 100644 --- a/internal/format/markdown_test.go +++ b/internal/format/markdown_test.go @@ -1109,6 +1109,29 @@ func TestFormatMarkdownImageInline(t *testing.T) { } } +func TestFormatMarkdownRenderedImageSkipsMarkdownImage(t *testing.T) { + p := core.TestTerminalPrinter(false) + err := FormatMarkdownWithOptions([]byte("Before ![logo](https://example.com/logo.png) after"), p, MarkdownOptions{ + RenderImage: func(destination string) bool { + if destination != "https://example.com/logo.png" { + t.Fatalf("image destination = %q", destination) + } + _, _ = p.WriteString("\n") + return true + }, + }) + if err != nil { + t.Fatalf("FormatMarkdownWithOptions() error = %v", err) + } + output := string(p.Bytes()) + if !strings.Contains(output, "") || !strings.Contains(output, "Before") || !strings.Contains(output, "after") { + t.Fatalf("rendered image output = %q", output) + } + if strings.Contains(output, "logo") || strings.Contains(output, "https://example.com/logo.png") { + t.Fatalf("Markdown image fallback was rendered: %q", output) + } +} + func TestFormatMarkdownTable(t *testing.T) { input := "| Name | Age |\n|------|-----|\n| Alice | 30 |\n| Bob | 25 |\n" p := core.TestPrinter(false) diff --git a/internal/image/block.go b/internal/image/block.go index 6d589f75..305a6067 100644 --- a/internal/image/block.go +++ b/internal/image/block.go @@ -21,8 +21,7 @@ type rgbColor struct { r, g, b int } -// writeBlocks resizes the image and outputs it as terminal blocks. -func writeBlocks(img image.Image, termWidth, termHeight int) error { +func writeBlocksTo(img image.Image, termWidth, termHeight int, writer io.Writer) error { trueColor := supportsTrueColor() // Each terminal block represents 2 vertical pixels. @@ -30,7 +29,7 @@ func writeBlocks(img image.Image, termWidth, termHeight int) error { targetWidth := cols targetHeight := rows * 2 - dst := resizeImage(img, targetWidth, targetHeight) + raster := resizeImage(img, targetWidth, targetHeight) // Process the image in blocks (each block = two vertical pixels). var out bytes.Buffer @@ -39,10 +38,10 @@ func writeBlocks(img image.Image, termWidth, termHeight int) error { bottomY := topY + 1 for x := range cols { - top := pixelToColor(dst.At(x, topY)) + top := pixelToColor(raster.At(x, topY)) var bottom *rgbColor if bottomY < targetHeight { - bottom = pixelToColor(dst.At(x, bottomY)) + bottom = pixelToColor(raster.At(x, bottomY)) } writeBlock(&out, top, bottom, trueColor) @@ -52,8 +51,8 @@ func writeBlocks(img image.Image, termWidth, termHeight int) error { // Reset ANSI formatting at the end. out.WriteString("\x1b[0m") - out.WriteTo(os.Stdout) - return nil + _, err := out.WriteTo(writer) + return err } // supportsTrueColor checks the current terminal emulator for true color support. diff --git a/internal/image/image.go b/internal/image/image.go index 2696c674..ca559688 100644 --- a/internal/image/image.go +++ b/internal/image/image.go @@ -9,7 +9,9 @@ import ( "image" _ "image/jpeg" "image/png" + "io" "math" + "os" "strings" "github.com/ryanfowler/fetch/internal/core" @@ -35,9 +37,19 @@ func Render(ctx context.Context, b []byte, nativeOnly bool) error { // explicit opt-in because image responses are untrusted input and adapters // have a much larger attack surface than the Go decoders. func RenderWithMode(ctx context.Context, b []byte, mode core.ImageSetting) error { + return RenderWithModeTo(ctx, b, mode, os.Stdout) +} + +// RenderWithModeTo decodes an image and writes its terminal presentation to +// dst. Keeping the destination explicit lets Markdown presentation buffer +// image protocols together with the surrounding document. +func RenderWithModeTo(ctx context.Context, b []byte, mode core.ImageSetting, dst io.Writer) error { if mode == core.ImageOff { return errors.New("image rendering is disabled") } + if dst == nil { + return errors.New("image rendering destination is nil") + } img, err := decodeImage(ctx, b, mode) if err != nil { return err @@ -57,16 +69,16 @@ func RenderWithMode(ctx context.Context, b []byte, mode core.ImageSetting) error if size.WidthPx == 0 || size.HeightPx == 0 { // If we're unable to get the terminal dimensions in pixels, // render the image using blocks. - return writeBlocks(img, size.Cols, size.Rows) + return writeBlocksTo(img, size.Cols, size.Rows, dst) } switch detectEmulator().Protocol() { case protoInline: - return writeInline(img, size.WidthPx, size.HeightPx) + return writeInlineTo(img, size.WidthPx, size.HeightPx, dst) case protoKitty: - return writeKitty(img, size.WidthPx, size.HeightPx) + return writeKittyTo(img, size.WidthPx, size.HeightPx, dst) default: - return writeBlocks(img, size.Cols, size.Rows) + return writeBlocksTo(img, size.Cols, size.Rows, dst) } } diff --git a/internal/image/inline.go b/internal/image/inline.go index ea732ed1..d139a412 100644 --- a/internal/image/inline.go +++ b/internal/image/inline.go @@ -3,12 +3,10 @@ package image import ( "fmt" "image" - "os" + "io" ) -// writeInline writes the provided image to the terminal using iTerm2's inline -// image protocol. -func writeInline(img image.Image, termWidthPx, termHeightPx int) error { +func writeInlineTo(img image.Image, termWidthPx, termHeightPx int, dst io.Writer) error { img = resizeForTerm(img, termWidthPx, termHeightPx) bounds := img.Bounds() width, height := bounds.Dx(), bounds.Dy() @@ -18,7 +16,7 @@ func writeInline(img image.Image, termWidthPx, termHeightPx int) error { return err } - fmt.Fprintf(os.Stdout, "\x1b]1337;File=inline=1;preserveAspectRatio=1;size=%d;width=%dpx;height=%dpx:%s\x07\n", + _, err = fmt.Fprintf(dst, "\x1b]1337;File=inline=1;preserveAspectRatio=1;size=%d;width=%dpx;height=%dpx:%s\x07\n", len(data), width, height, data) - return nil + return err } diff --git a/internal/image/kitty.go b/internal/image/kitty.go index 62ae6902..2f01e10b 100644 --- a/internal/image/kitty.go +++ b/internal/image/kitty.go @@ -3,12 +3,10 @@ package image import ( "fmt" "image" - "os" + "io" ) -// writeKitty writes the provided image to the terminal using the kitty -// graphics protocol. -func writeKitty(img image.Image, termWidthPx, termHeightPx int) error { +func writeKittyTo(img image.Image, termWidthPx, termHeightPx int, dst io.Writer) error { img = resizeForTerm(img, termWidthPx, termHeightPx) bounds := img.Bounds() width, height := bounds.Dx(), bounds.Dy() @@ -21,8 +19,10 @@ func writeKitty(img image.Image, termWidthPx, termHeightPx int) error { // The image is written in chunks of up to 4096 bytes. next := min(4096, len(data)) chunk := data[:next] - fmt.Fprintf(os.Stdout, "\x1b_Gq=2,f=100,a=T,t=d,s=%d,v=%d,m=%d;%s\x1b\\", - width, height, boolToInt(next < len(data)), chunk) + if _, err := fmt.Fprintf(dst, "\x1b_Gq=2,f=100,a=T,t=d,s=%d,v=%d,m=%d;%s\x1b\\", + width, height, boolToInt(next < len(data)), chunk); err != nil { + return err + } pos := next for pos < len(data) { @@ -30,12 +30,14 @@ func writeKitty(img image.Image, termWidthPx, termHeightPx int) error { chunk = data[pos:next] pos = next - fmt.Fprintf(os.Stdout, "\x1b_Gm=%d;%s\x1b\\", - boolToInt(next < len(data)), chunk) + if _, err := fmt.Fprintf(dst, "\x1b_Gm=%d;%s\x1b\\", + boolToInt(next < len(data)), chunk); err != nil { + return err + } } - fmt.Fprintln(os.Stdout) - return nil + _, err = fmt.Fprintln(dst) + return err } func boolToInt(b bool) int {