Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PDF files not showing in renderer #8294

Closed
1 of 7 tasks
mehalter opened this issue Sep 26, 2019 · 11 comments
Closed
1 of 7 tasks

PDF files not showing in renderer #8294

mehalter opened this issue Sep 26, 2019 · 11 comments
Labels
issue/stale type/question Issue needs no code to be fixed, only a description on how to fix it yourself.

Comments

@mehalter
Copy link

  • Gitea version (or commit ref): 1.9.3
  • Git version: 2.16.5
  • Operating system: CentOS
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

PDF Files are not showing in the renderer, the /vendor/plugins/pdfjs/web/viewer.html?file=...pdf request has X-Frame-Options: DENY header. I can follow the link and serve the file, but the header option doesn't allow it to be in the iframe. When I set the headers in my apache config, it doesn't apply to the /vendor/plugins/pdfjs/web/viewer.html?file=....pdf requests.

Screenshots

image

@HarvsG
Copy link
Contributor

HarvsG commented Sep 27, 2019

As I found in #8299 some external rendering was broken by 867f46f, not sure if it would have affected PDF files, but may be worth checking.

It looks as if PDFs are handled in different cases in the switch:

gitea/routers/repo/view.go

Lines 274 to 345 in c6fb7fe

switch {
case isTextFile:
if fileSize >= setting.UI.MaxDisplayFileSize {
ctx.Data["IsFileTooLarge"] = true
break
}
d, _ := ioutil.ReadAll(dataRc)
buf = charset.ToUTF8WithFallback(append(buf, d...))
readmeExist := markup.IsReadmeFile(blob.Name())
ctx.Data["ReadmeExist"] = readmeExist
if markupType := markup.Type(blob.Name()); markupType != "" {
ctx.Data["IsMarkup"] = true
ctx.Data["MarkupType"] = markupType
ctx.Data["FileContent"] = string(markup.Render(blob.Name(), buf, path.Dir(treeLink), ctx.Repo.Repository.ComposeMetas()))
} else if readmeExist {
ctx.Data["IsRenderedHTML"] = true
ctx.Data["FileContent"] = strings.Replace(
gotemplate.HTMLEscapeString(string(buf)), "\n", `<br>`, -1,
)
} else {
// Building code view blocks with line number on server side.
var fileContent string
if content, err := charset.ToUTF8WithErr(buf); err != nil {
log.Error("ToUTF8WithErr: %v", err)
fileContent = string(buf)
} else {
fileContent = content
}
var output bytes.Buffer
lines := strings.Split(fileContent, "\n")
//Remove blank line at the end of file
if len(lines) > 0 && lines[len(lines)-1] == "" {
lines = lines[:len(lines)-1]
}
for index, line := range lines {
line = gotemplate.HTMLEscapeString(line)
if index != len(lines)-1 {
line += "\n"
}
output.WriteString(fmt.Sprintf(`<li class="L%d" rel="L%d">%s</li>`, index+1, index+1, line))
}
ctx.Data["FileContent"] = gotemplate.HTML(output.String())
output.Reset()
for i := 0; i < len(lines); i++ {
output.WriteString(fmt.Sprintf(`<span id="L%[1]d" data-line-number="%[1]d"></span>`, i+1))
}
ctx.Data["LineNums"] = gotemplate.HTML(output.String())
}
if !isLFSFile {
if ctx.Repo.CanEnableEditor() {
ctx.Data["CanEditFile"] = true
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file")
} else if !ctx.Repo.IsViewBranch {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch")
} else if !ctx.Repo.CanWrite(models.UnitTypeCode) {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit")
}
}
case base.IsPDFFile(buf):
ctx.Data["IsPDFFile"] = true
case base.IsVideoFile(buf):
ctx.Data["IsVideoFile"] = true
case base.IsAudioFile(buf):
ctx.Data["IsAudioFile"] = true
case base.IsImageFile(buf):
ctx.Data["IsImageFile"] = true
}

Once the case is handled it is processed by the template at:

<div class="file-view {{if .IsMarkup}}{{.MarkupType}}{{else if .IsRenderedHTML}}plain-text{{else if .IsTextFile}}code-view{{end}} has-emoji">
{{if .IsMarkup}}
{{if .FileContent}}{{.FileContent | Safe}}{{end}}
{{else if .IsRenderedHTML}}
<pre>{{if .FileContent}}{{.FileContent | Str2html}}{{end}}</pre>
{{else if not .IsTextFile}}
<div class="view-raw ui center">
{{if .IsImageFile}}
<img src="{{EscapePound $.RawFileLink}}">
{{else if .IsVideoFile}}
<video controls src="{{EscapePound $.RawFileLink}}">
<strong>{{.i18n.Tr "repo.video_not_supported_in_browser"}}</strong>
</video>
{{else if .IsAudioFile}}
<audio controls src="{{EscapePound $.RawFileLink}}">
<strong>{{.i18n.Tr "repo.audio_not_supported_in_browser"}}</strong>
</audio>
{{else if .IsPDFFile}}
<iframe width="100%" height="600px" src="{{AppSubUrl}}/vendor/plugins/pdfjs/web/viewer.html?file={{EscapePound $.RawFileLink}}"></iframe>
{{else}}
<a href="{{EscapePound $.RawFileLink}}" rel="nofollow" class="btn btn-gray btn-radius">{{.i18n.Tr "repo.file_view_raw"}}</a>
{{end}}
</div>

Edit: However, unhelpfully, I tested it on my install of master and had no problems:
Screenshot 2019-09-27 at 13 55 02

@mehalter
Copy link
Author

Yeah the generated HTML looks good and seems ok. I'm trying to figure out why the headers are setting X-Frame-Options to DENY instead of SAMEORIGIN only for the PDF.js links. Do you have any idea why this would be different than the headers of the rest of the site?

@HarvsG
Copy link
Contributor

HarvsG commented Sep 27, 2019

@mehalter Since I'm not having any issues on my side I wonder if it could be your browser? Have you tried from different browsers/clients? Some browsers/browser extensions make changes that interfere with iframes for security reasons.

@mehalter
Copy link
Author

Yeah I had that thought too, and it is broken on all browsers/clients that I have tried on several devices.

@mehalter
Copy link
Author

Is there anything in the codebase that would have the pdf.js link be assigned different headers? the /vendor/plugins/pdfjs/web/viewer.html?file=....pdf link? All my other renderer file types work just fine. But the pdfjs links in particular are getting assigned this header.

@lunny lunny added the type/question Issue needs no code to be fixed, only a description on how to fix it yourself. label Sep 30, 2019
@lunny
Copy link
Member

lunny commented Sep 30, 2019

Which browser are you using?

@mehalter
Copy link
Author

I have tried with Firefox and qutebrowser on linux, and firefox and chrome on android.

@guillep2k
Copy link
Member

@mehalter #8300 has some code changes about rendering external files. Could that fix your problem? You can checkout that PR by using:

pull=8300
git fetch upstream "pull/$pull/head:pull-$pull"
git checkout "pull-$pull"

But you need to check in a testing environment; otherwise it might wreck havoc in your installation (there was a bug in migrations that deleted the attachments from the file system and I don't know if the fix has been merged into this particular PR yet).

@guillep2k
Copy link
Member

external files. Could that fix your problem? You can checkout that PR by using:

Or perhaps you could attempt applying that commit individually? It would be safer and you'd skip migrations, etc.

@stale
Copy link

stale bot commented Dec 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Dec 9, 2019
@stale
Copy link

stale bot commented Dec 23, 2019

This issue has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this as completed Dec 23, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale type/question Issue needs no code to be fixed, only a description on how to fix it yourself.
Projects
None yet
Development

No branches or pull requests

4 participants