search

lukasjarosch / go-docx

Replace placeholders inside docx documents with speed and confidence.
MIT License
211 stars 47 forks source link

Panic in ParsePlaceholders function in placeholders.go #29

Open kavinsan opened 3 months ago

kavinsan commented 3 months ago

I've came across a panic caused by an unsafe slice access

image

`panic: runtime error: index out of range [0] with length 0

goroutine 42 [running]: github.com/lukasjarosch/go-docx.assembleFullPlaceholders(0xc00056abe0, {0xc000764500, 0x1, 0x2}, {0x0, 0x0, 0x0}) /Users/kav/go/pkg/mod/github.com/lukasjarosch/go-docx@v0.5.0/placeholder.go:272 +0x285 github.com/lukasjarosch/go-docx.ParsePlaceholders({0xc00058e000, 0x112, 0x200}, {0xc0005ee000, 0xc741, 0xe000}) /Users/kav/go/pkg/mod/github.com/lukasjarosch/go-docx@v0.5.0/placeholder.go:196 +0xf4c github.com/lukasjarosch/go-docx.newDocument(0xc000488508, {0xc0001802a0, 0x57}, 0xc0001916f0) /Users/kav/go/pkg/mod/github.com/lukasjarosch/go-docx@v0.5.0/document.go:126 +0x7a7 github.com/lukasjarosch/go-docx.Open({0xc0001802a0, 0x57}) /Users/kav/go/pkg/mod/github.com/lukasjarosch/go-docx@v0.5.0/document.go:70 +0x405 main.DocxToPdfClient.InsertDataIntoDocument({}, 0xc0005d82a0, {0x0, 0x0, 0x0}, {0xc0001802a0, 0x57}) /Users/kav/go/src/github.com/EnergyWell/pdf-service/app/docx_to_pdf.go:33 +0x2cf main.(MergeFilesToPdfClient).ConvertNonPdfSourceFilesToPdf(0xc00002ba28, {0x24b6590, 0x2c2e740}) /Users/kav/go/src/github.com/EnergyWell/pdf-service/app/merge_pdfs.go:104 +0x1f0 main.(service).MergeFilesToPdf(0xc0001080f0, {0x24ba590, 0xc0005d8a80}, 0xc0001dc3f0) /Users/kav/go/src/github.com/EnergyWell/pdf-service/app/pdfservice.go:198 +0xd3e github.com/EnergyWell/pdf-service/proto._PdfService_MergeFilesToPdf_Handler.func1({0x24ba590, 0xc0005d8a80}, {0x214f740, 0xc0001dc3f0}) /Users/kav/go/src/github.com/EnergyWell/pdf-service/proto/pdf_generator.pb.go:958 +0xf2 github.com/mwitkow/go-grpc-middleware.ChainUnaryServer.func1.1({0x24ba590, 0xc0005d8a80}, {0x214f740, 0xc0001dc3f0}) /Users/kav/go/pkg/mod/github.com/mwitkow/go-grpc-middleware@v1.0.0/chain.go:31 +0xea github.com/EnergyWell/authorization.(authorizer).authorizeAdminOnly(0xc000108140, {0x24ba590, 0xc0005d8a80}, {0x214f740, 0xc0001dc3f0}, 0xc00052e520, 0xc0001f48c0) /Users/kav/go/pkg/mod/github.com/!energy!well/authorization@v0.0.0-20220817035557-04fac2b853fe/authorization.go:132 +0x1a4 github.com/EnergyWell/authorization.(authorizer).Middleware(0xc000108140, {0x24ba590, 0xc0005d8a80}, {0x214f740, 0xc0001dc3f0}, 0xc00052e520, 0xc0001f48c0) /Users/kav/go/pkg/mod/github.com/!energy!well/authorization@v0.0.0-20220817035557-04fac2b853fe/authorization.go:68 +0x447 github.com/mwitkow/go-grpc-middleware.ChainUnaryServer.func1.1({0x24ba590, 0xc0005d8a80}, {0x214f740, 0xc0001dc3f0}) /Users/kav/go/pkg/mod/github.com/mwitkow/go-grpc-middleware@v1.0.0/chain.go:34 +0x212 github.com/EnergyWell/auth.(AuthMiddleware).UnaryMiddleware(0xc0003d4f20, {0x24ba590, 0xc0005d8a80}, {0x214f740, 0xc0001dc3f0}, 0xc00052e520, 0xc0001f48c0) /Users/kav/go/pkg/mod/github.com/!energy!well/auth@v0.0.0-20220811211414-48e96e06b0ef/middleware.go:77 +0x51a github.com/mwitkow/go-grpc-middleware.ChainUnaryServer.func1.1({0x24ba590, 0xc0005d8300}, {0x214f740, 0xc0001dc3f0}) /Users/kav/go/pkg/mod/github.com/mwitkow/go-grpc-middleware@v1.0.0/chain.go:34 +0x212 github.com/EnergyWell/service/logger.UnaryMiddleware({0x24ba590, 0xc0005d8300}, {0x214f740, 0xc0001dc3f0}, 0xc00052e520, 0xc0001f48c0) /Users/kav/go/pkg/mod/github.com/!energy!well/service@v0.0.0-20220127220047-85a8bafcef39/logger/middleware.go:39 +0x3e6 github.com/mwitkow/go-grpc-middleware.ChainUnaryServer.func1({0x24ba590, 0xc0005d8270}, {0x214f740, 0xc0001dc3f0}, 0xc00052e520, 0xc0000121f8) /Users/kav/go/pkg/mod/github.com/mwitkow/go-grpc-middleware@v1.0.0/chain.go:39 +0x2a6 github.com/EnergyWell/pdf-service/proto._PdfService_MergeFilesToPdf_Handler({0x20f0900, 0xc0001080f0}, {0x24ba590, 0xc0005d8270}, 0xc0001aa420, 0xc000446450) /Users/kav/go/src/github.com/EnergyWell/pdf-service/proto/pdf_generator.pb.go:960 +0x216 google.golang.org/grpc.(Server).processUnaryRPC(0xc00044c000, {0x24bdcb8, 0xc0004d1520}, 0xc000504000, 0xc0004762a0, 0x2bec360, 0x0) /Users/kav/go/pkg/mod/google.golang.org/grpc@v1.44.0/server.go:1282 +0x15e3 google.golang.org/grpc.(Server).handleStream(0xc00044c000, {0x24bdcb8, 0xc0004d1520}, 0xc000504000, 0x0) /Users/kav/go/pkg/mod/google.golang.org/grpc@v1.44.0/server.go:1616 +0x891 google.golang.org/grpc.(Server).serveStreams.func1.2() /Users/kav/go/pkg/mod/google.golang.org/grpc@v1.44.0/server.go:921 +0x117 created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 57 /Users/kav/go/pkg/mod/google.golang.org/grpc@v1.44.0/server.go:919 +0x385`

lukasjarosch commented 2 months ago

Thanks for reporting this, I'll check it out after work 👍🏼

PanicIsReal commented 2 months ago

Any idea what may be causing this? I am experiencing the same issue.

kavinsan commented 2 months ago

Any idea what may be causing this? I am experiencing the same issue.

For me it was using a template with the wrong placeholder. Note the extra {} Ex.

Hopefully we get a patch for this soon! :)

TeCHiScy commented 2 months ago

Hopefully we get a patch for this soon! :)

I will take a look at this too.

Update 1: I have reproduced the panic. The essential of the panic is that in a Run, there's more than one openDelims, but no closeDelims (or may be len(openDelims) > len(closeDelims) + 1). I created two cases, as below:

  1. The template provided is corrupted, e.g. {{holder,
  2. The template was acceptable, but the Run was divided, e.g. {{{holder}}}, and in the xml it was divided as {{|{holder}|}}. Here, | represents the divider.

Update 2: A PR that refactors ParsePlaceholders to avoid the aforesaid panic (as well as make itself more concise and adaptable) is work in process. This refactor may also lay a foundation for #28. I'm testing the new function. Once the PR is ready, I'll update here.

Update 3: Hi @PanicIsReal, @kavinsan, could you please attach a minimal reproducible example (i.e., your docx and the replace map) here, so I can use this as test cases to see if my PR #32 works well now.

Update 4: Unresolved issue, if a placeholder cross table cells (or any hard boundaries that should not be crossed), it is also replaced. (if allowed, then it may be messed up when some kind of template engine introduced). Besides, this weird behavior in fact relies on the order of runs in xml, which is unexpected to users.