DRY blob excerpt: unified single/batch path, single-pass highlighting

- Merge ExcerptBlob and excerptBlobBatch into a single unified flow
  that handles both single and batch requests without duplicated code
- Extract parseBatchBlobExcerptOptions to DRY the repetitive splitInts calls
- Refactor BuildBlobExcerptDiffSection to separate section building from
  highlighting, enabling BuildBlobExcerptDiffSections (plural) to highlight
  the file content only once for all sections instead of N times
- Add blob size limit check (MaxDisplayFileSize) before io.ReadAll to
  prevent OOM on large files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

routers/web/repo/compare.go

@@ -4,7 +4,6 @@
 package repo
 
 import (
-	"bytes"
 	gocontext "context"
 	"encoding/csv"
 	"errors"
@@ -765,17 +764,8 @@ func splitInts(s string) ([]int, error) {
 // ExcerptBlob render blob excerpt contents
 func ExcerptBlob(ctx *context.Context) {
 	commitID := ctx.PathParam("sha")
-	opts := gitdiff.BlobExcerptOptions{
-		LastLeft:      ctx.FormInt("last_left"),
-		LastRight:     ctx.FormInt("last_right"),
-		LeftIndex:     ctx.FormInt("left"),
-		RightIndex:    ctx.FormInt("right"),
-		LeftHunkSize:  ctx.FormInt("left_hunk_size"),
-		RightHunkSize: ctx.FormInt("right_hunk_size"),
-		Direction:     ctx.FormString("direction"),
-		Language:      ctx.FormString("filelang"),
-	}
 	filePath := ctx.FormString("path")
+	language := ctx.FormString("filelang")
 	gitRepo := ctx.Repo.GitRepo
 
 	diffBlobExcerptData := &gitdiff.DiffBlobExcerptData{
@@ -794,11 +784,28 @@ func ExcerptBlob(ctx *context.Context) {
 		diffBlobExcerptData.BaseLink = ctx.Repo.RepoLink + "/wiki/blob_excerpt"
 	}
 
-	// Batch mode: if last_left contains a comma, treat all per-gap params as
-	// comma-separated lists and return a JSON array of HTML strings.
-	if strings.Contains(ctx.FormString("last_left"), ",") {
-		excerptBlobBatch(ctx, gitRepo, commitID, filePath, opts.Language, diffBlobExcerptData)
-		return
+	// Detect batch mode: comma in last_left means comma-separated arrays
+	isBatch := strings.Contains(ctx.FormString("last_left"), ",")
+
+	// Parse options: batch parses comma-separated arrays, single parses individual values
+	var optsList []gitdiff.BlobExcerptOptions
+	if isBatch {
+		var ok bool
+		optsList, ok = parseBatchBlobExcerptOptions(ctx, language)
+		if !ok {
+			return
+		}
+	} else {
+		optsList = []gitdiff.BlobExcerptOptions{{
+			LastLeft:      ctx.FormInt("last_left"),
+			LastRight:     ctx.FormInt("last_right"),
+			LeftIndex:     ctx.FormInt("left"),
+			RightIndex:    ctx.FormInt("right"),
+			LeftHunkSize:  ctx.FormInt("left_hunk_size"),
+			RightHunkSize: ctx.FormInt("right_hunk_size"),
+			Direction:     ctx.FormString("direction"),
+			Language:      language,
+		}}
 	}
 
 	commit, err := gitRepo.GetCommit(commitID)
@@ -811,26 +818,35 @@ func ExcerptBlob(ctx *context.Context) {
 		ctx.ServerError("GetBlobByPath", err)
 		return
 	}
+	if blob.Size() > setting.UI.MaxDisplayFileSize {
+		ctx.HTTPError(http.StatusRequestEntityTooLarge, "blob too large for expansion")
+		return
+	}
 	reader, err := blob.DataAsync()
 	if err != nil {
 		ctx.ServerError("DataAsync", err)
 		return
 	}
-	defer reader.Close()
-
-	section, err := gitdiff.BuildBlobExcerptDiffSection(filePath, reader, opts)
+	blobData, err := io.ReadAll(reader)
+	reader.Close()
 	if err != nil {
-		ctx.ServerError("BuildBlobExcerptDiffSection", err)
+		ctx.ServerError("ReadAll", err)
 		return
 	}
 
+	sections, err := gitdiff.BuildBlobExcerptDiffSections(filePath, blobData, optsList)
+	if err != nil {
+		ctx.ServerError("BuildBlobExcerptDiffSections", err)
+		return
+	}
+
+	// Fetch PR comments and attach to sections
 	diffBlobExcerptData.PullIssueIndex = ctx.FormInt64("pull_issue_index")
 	if diffBlobExcerptData.PullIssueIndex > 0 {
 		if !ctx.Repo.CanRead(unit.TypePullRequests) {
 			ctx.NotFound(nil)
 			return
 		}
-
 		issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, diffBlobExcerptData.PullIssueIndex)
 		if err != nil {
 			log.Error("GetIssueByIndex error: %v", err)
@@ -847,8 +863,8 @@ func ExcerptBlob(ctx *context.Context) {
 			allComments, err := issues_model.FetchCodeComments(ctx, issue, ctx.Doer, ctx.FormBool("show_outdated"))
 			if err != nil {
 				log.Error("FetchCodeComments error: %v", err)
-			} else {
-				if lineComments, ok := allComments[filePath]; ok {
+			} else if lineComments, ok := allComments[filePath]; ok {
+				for _, section := range sections {
 					attachCommentsToLines(section, lineComments)
 					attachHiddenCommentIDs(section, lineComments)
 				}
@@ -856,134 +872,62 @@ func ExcerptBlob(ctx *context.Context) {
 		}
 	}
 
-	ctx.Data["section"] = section
 	ctx.Data["FileNameHash"] = git.HashFilePathForWebUI(filePath)
 	ctx.Data["DiffBlobExcerptData"] = diffBlobExcerptData
 
-	ctx.HTML(http.StatusOK, tplBlobExcerpt)
+	// Respond: single returns HTML, batch returns JSON array of HTML strings
+	if isBatch {
+		htmlStrings := make([]string, len(sections))
+		for i, section := range sections {
+			ctx.Data["section"] = section
+			html, err := ctx.RenderToHTML(tplBlobExcerpt, ctx.Data)
+			if err != nil {
+				ctx.ServerError("RenderToHTML", err)
+				return
+			}
+			htmlStrings[i] = string(html)
+		}
+		ctx.JSON(http.StatusOK, htmlStrings)
+	} else {
+		ctx.Data["section"] = sections[0]
+		ctx.HTML(http.StatusOK, tplBlobExcerpt)
+	}
 }
 
-func excerptBlobBatch(ctx *context.Context, gitRepo *git.Repository, commitID, filePath, language string, diffBlobExcerptData *gitdiff.DiffBlobExcerptData) {
-	lastLefts, err := splitInts(ctx.FormString("last_left"))
-	if err != nil {
-		ctx.HTTPError(http.StatusBadRequest, "invalid last_left values")
-		return
-	}
-	lastRights, err := splitInts(ctx.FormString("last_right"))
-	if err != nil {
-		ctx.HTTPError(http.StatusBadRequest, "invalid last_right values")
-		return
-	}
-	lefts, err := splitInts(ctx.FormString("left"))
-	if err != nil {
-		ctx.HTTPError(http.StatusBadRequest, "invalid left values")
-		return
-	}
-	rights, err := splitInts(ctx.FormString("right"))
-	if err != nil {
-		ctx.HTTPError(http.StatusBadRequest, "invalid right values")
-		return
-	}
-	leftHunkSizes, err := splitInts(ctx.FormString("left_hunk_size"))
-	if err != nil {
-		ctx.HTTPError(http.StatusBadRequest, "invalid left_hunk_size values")
-		return
-	}
-	rightHunkSizes, err := splitInts(ctx.FormString("right_hunk_size"))
-	if err != nil {
-		ctx.HTTPError(http.StatusBadRequest, "invalid right_hunk_size values")
-		return
-	}
-
-	n := len(lastLefts)
-	if len(lastRights) != n || len(lefts) != n || len(rights) != n || len(leftHunkSizes) != n || len(rightHunkSizes) != n {
-		ctx.HTTPError(http.StatusBadRequest, "all per-gap parameter arrays must have the same length")
-		return
-	}
-
-	commit, err := gitRepo.GetCommit(commitID)
-	if err != nil {
-		ctx.ServerError("GetCommit", err)
-		return
-	}
-	blob, err := commit.Tree.GetBlobByPath(filePath)
-	if err != nil {
-		ctx.ServerError("GetBlobByPath", err)
-		return
-	}
-	reader, err := blob.DataAsync()
-	if err != nil {
-		ctx.ServerError("DataAsync", err)
-		return
-	}
-	blobData, err := io.ReadAll(reader)
-	reader.Close()
-	if err != nil {
-		ctx.ServerError("ReadAll", err)
-		return
-	}
-
-	diffBlobExcerptData.PullIssueIndex = ctx.FormInt64("pull_issue_index")
-	var lineComments map[int64][]*issues_model.Comment
-	if diffBlobExcerptData.PullIssueIndex > 0 {
-		if !ctx.Repo.CanRead(unit.TypePullRequests) {
-			ctx.NotFound(nil)
-			return
-		}
-		issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, diffBlobExcerptData.PullIssueIndex)
+// parseBatchBlobExcerptOptions parses comma-separated per-gap parameters for batch expansion.
+// Returns false if an error response has been sent.
+func parseBatchBlobExcerptOptions(ctx *context.Context, language string) ([]gitdiff.BlobExcerptOptions, bool) {
+	paramNames := [6]string{"last_left", "last_right", "left", "right", "left_hunk_size", "right_hunk_size"}
+	var parsed [6][]int
+	for i, name := range paramNames {
+		vals, err := splitInts(ctx.FormString(name))
 		if err != nil {
-			log.Error("GetIssueByIndex error: %v", err)
-		} else if issue.IsPull {
-			ctx.Data["Issue"] = issue
-			ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
-				return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
-			}
-			ctx.Data["PageIsPullFiles"] = true
-			ctx.Data["AfterCommitID"] = diffBlobExcerptData.AfterCommitID
-			allComments, err := issues_model.FetchCodeComments(ctx, issue, ctx.Doer, ctx.FormBool("show_outdated"))
-			if err != nil {
-				log.Error("FetchCodeComments error: %v", err)
-			} else {
-				lineComments = allComments[filePath]
-			}
+			ctx.HTTPError(http.StatusBadRequest, "invalid "+name+" values")
+			return nil, false
+		}
+		parsed[i] = vals
+	}
+
+	n := len(parsed[0])
+	for i := 1; i < len(parsed); i++ {
+		if len(parsed[i]) != n {
+			ctx.HTTPError(http.StatusBadRequest, "all per-gap parameter arrays must have the same length")
+			return nil, false
 		}
 	}
 
-	ctx.Data["FileNameHash"] = git.HashFilePathForWebUI(filePath)
-	ctx.Data["DiffBlobExcerptData"] = diffBlobExcerptData
-
-	htmlStrings := make([]string, n)
+	optsList := make([]gitdiff.BlobExcerptOptions, n)
 	for i := range n {
-		opts := gitdiff.BlobExcerptOptions{
-			LastLeft:      lastLefts[i],
-			LastRight:     lastRights[i],
-			LeftIndex:     lefts[i],
-			RightIndex:    rights[i],
-			LeftHunkSize:  leftHunkSizes[i],
-			RightHunkSize: rightHunkSizes[i],
+		optsList[i] = gitdiff.BlobExcerptOptions{
+			LastLeft:      parsed[0][i],
+			LastRight:     parsed[1][i],
+			LeftIndex:     parsed[2][i],
+			RightIndex:    parsed[3][i],
+			LeftHunkSize:  parsed[4][i],
+			RightHunkSize: parsed[5][i],
 			Direction:     "full",
 			Language:      language,
 		}
-
-		section, err := gitdiff.BuildBlobExcerptDiffSection(filePath, bytes.NewReader(blobData), opts)
-		if err != nil {
-			ctx.ServerError("BuildBlobExcerptDiffSection", err)
-			return
-		}
-
-		if lineComments != nil {
-			attachCommentsToLines(section, lineComments)
-			attachHiddenCommentIDs(section, lineComments)
-		}
-
-		ctx.Data["section"] = section
-		html, err := ctx.RenderToHTML(tplBlobExcerpt, ctx.Data)
-		if err != nil {
-			ctx.ServerError("RenderToHTML", err)
-			return
-		}
-		htmlStrings[i] = string(html)
 	}
-
-	ctx.JSON(http.StatusOK, htmlStrings)
+	return optsList, true
 }

services/gitdiff/gitdiff_excerpt.go

@@ -26,7 +26,9 @@ type BlobExcerptOptions struct {
 	Language      string
 }
 
-func fillExcerptLines(section *DiffSection, filePath string, reader io.Reader, lang string, idxLeft, idxRight, chunkSize int) error {
+// fillExcerptLines reads from reader and populates section.Lines.
+// It returns the accumulated content buffer for later highlighting.
+func fillExcerptLines(section *DiffSection, reader io.Reader, idxLeft, idxRight, chunkSize int) ([]byte, error) {
 	buf := &bytes.Buffer{}
 	scanner := bufio.NewScanner(reader)
 	var diffLines []*DiffLine
@@ -51,36 +53,36 @@ func fillExcerptLines(section *DiffSection, filePath string, reader io.Reader, l
 		diffLines = append(diffLines, diffLine)
 	}
 	if err := scanner.Err(); err != nil {
-		return fmt.Errorf("fillExcerptLines scan: %w", err)
+		return nil, fmt.Errorf("fillExcerptLines scan: %w", err)
 	}
 	section.Lines = diffLines
-	// DiffLinePlain always uses right lines
-	section.highlightedRightLines.value = highlightCodeLines(filePath, lang, []*DiffSection{section}, false /* right */, buf.Bytes())
-	return nil
+	return buf.Bytes(), nil
 }
 
-func BuildBlobExcerptDiffSection(filePath string, reader io.Reader, opts BlobExcerptOptions) (*DiffSection, error) {
+// buildExcerptDiffSection builds a single excerpt section without highlighting.
+// It returns the section and the accumulated content buffer.
+func buildExcerptDiffSection(filePath string, reader io.Reader, opts BlobExcerptOptions) (*DiffSection, []byte, error) {
 	lastLeft, lastRight, idxLeft, idxRight := opts.LastLeft, opts.LastRight, opts.LeftIndex, opts.RightIndex
 	leftHunkSize, rightHunkSize, direction := opts.LeftHunkSize, opts.RightHunkSize, opts.Direction
-	language := opts.Language
 
 	chunkSize := BlobExcerptChunkSize
 	section := &DiffSection{
-		language:              &diffVarMutable[string]{value: language},
+		language:              &diffVarMutable[string]{value: opts.Language},
 		highlightLexer:        &diffVarMutable[chroma.Lexer]{},
 		highlightedLeftLines:  &diffVarMutable[map[int]template.HTML]{},
 		highlightedRightLines: &diffVarMutable[map[int]template.HTML]{},
 		FileName:              filePath,
 	}
+	var bufContent []byte
 	var err error
 	if direction == "up" && (idxLeft-lastLeft) > chunkSize {
 		idxLeft -= chunkSize
 		idxRight -= chunkSize
 		leftHunkSize += chunkSize
 		rightHunkSize += chunkSize
-		err = fillExcerptLines(section, filePath, reader, language, idxLeft-1, idxRight-1, chunkSize)
+		bufContent, err = fillExcerptLines(section, reader, idxLeft-1, idxRight-1, chunkSize)
 	} else if direction == "down" && (idxLeft-lastLeft) > chunkSize {
-		err = fillExcerptLines(section, filePath, reader, language, lastLeft, lastRight, chunkSize)
+		bufContent, err = fillExcerptLines(section, reader, lastLeft, lastRight, chunkSize)
 		lastLeft += chunkSize
 		lastRight += chunkSize
 	} else {
@@ -88,14 +90,14 @@ func BuildBlobExcerptDiffSection(filePath string, reader io.Reader, opts BlobExc
 		if direction == "down" {
 			offset = 0
 		}
-		err = fillExcerptLines(section, filePath, reader, language, lastLeft, lastRight, idxRight-lastRight+offset)
+		bufContent, err = fillExcerptLines(section, reader, lastLeft, lastRight, idxRight-lastRight+offset)
 		leftHunkSize = 0
 		rightHunkSize = 0
 		idxLeft = lastLeft
 		idxRight = lastRight
 	}
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	newLineSection := &DiffLine{
@@ -120,5 +122,39 @@ func BuildBlobExcerptDiffSection(filePath string, reader io.Reader, opts BlobExc
 			section.Lines = append(section.Lines, newLineSection)
 		}
 	}
+	return section, bufContent, nil
+}
+
+// BuildBlobExcerptDiffSection builds a single excerpt section with highlighting.
+func BuildBlobExcerptDiffSection(filePath string, reader io.Reader, opts BlobExcerptOptions) (*DiffSection, error) {
+	section, bufContent, err := buildExcerptDiffSection(filePath, reader, opts)
+	if err != nil {
+		return nil, err
+	}
+	// DiffLinePlain always uses right lines
+	section.highlightedRightLines.value = highlightCodeLines(filePath, opts.Language, []*DiffSection{section}, false /* right */, bufContent)
 	return section, nil
 }
+
+// BuildBlobExcerptDiffSections builds multiple excerpt sections from the same file content,
+// highlighting the content only once for all sections.
+func BuildBlobExcerptDiffSections(filePath string, content []byte, optsList []BlobExcerptOptions) ([]*DiffSection, error) {
+	sections := make([]*DiffSection, len(optsList))
+	for i, opts := range optsList {
+		section, _, err := buildExcerptDiffSection(filePath, bytes.NewReader(content), opts)
+		if err != nil {
+			return nil, err
+		}
+		sections[i] = section
+	}
+
+	// Highlight once for all sections
+	if len(optsList) > 0 {
+		highlighted := highlightCodeLines(filePath, optsList[0].Language, sections, false /* right */, content)
+		for _, section := range sections {
+			section.highlightedRightLines.value = highlighted
+		}
+	}
+
+	return sections, nil
+}