Copilot commented on code in PR #1500:
URL: https://github.com/apache/airflow-site/pull/1500#discussion_r3089364725
##########
landing-pages/site/layouts/blog/list.xml:
##########
@@ -15,29 +15,29 @@
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-*/}}
-
-<?xml version="1.0" encoding="utf-8"?>
+*/ -}}
+{{- $base := .Site.Params.canonicalBaseURL | default "" -}}
+{{ printf "<?xml version=\"1.0\" encoding=\"utf-8\"?>" | safeHTML }}
<feed xmlns="http://www.w3.org/2005/Atom";>
<title>{{ .Site.Title }} Blog</title>
- <link href="{{ .Permalink }}" rel="alternate"/>
-<link href="{{ .Permalink }}index.xml" rel="self" type="application/atom+xml"/>
-<id>{{ .Permalink }}</id>
+ <link href="{{ $base }}{{ .Permalink }}" rel="alternate"/>
+ <link href="{{ $base }}{{ .Permalink }}index.xml" rel="self"
type="application/atom+xml"/>
+ <id>{{ $base }}{{ .Permalink }}</id>
{{ $pages := sort .Pages "Lastmod" "desc" }}
{{ with index $pages 0 }}
<updated>{{ .Lastmod.Format "2006-01-02T15:04:05Z" }}</updated>
{{ end }}
{{ range sort .Pages "Lastmod" "desc" }}
<entry>
<title>{{ .Title }}</title>
- <link href="{{ .Permalink }}" rel="alternate"/>
- <id>{{ .Permalink }}</id>
+ <link href="{{ $base }}{{ .Permalink }}" rel="alternate"/>
+ <id>{{ $base }}{{ .Permalink }}</id>
<published>{{ .PublishDate.Format "2006-01-02T15:04:05Z" }}</published>
<updated>{{ .Lastmod.Format "2006-01-02T15:04:05Z" }}</updated>
<author>
<name>Apache Airflow</name>
</author>
- <content type="html"><![CDATA[{{ .Content }}]]></content>
+ <content type="html">{{ printf "<![CDATA[%s]]>" .Content | safeHTML
}}</content>
Review Comment:
Wrapping `.Content` in a single CDATA section will still generate invalid
XML if the rendered HTML ever contains the sequence `]]>` (which terminates
CDATA). To keep the feed robust, consider splitting/escaping that sequence
before emitting CDATA (or alternatively avoid CDATA and XML-escape the HTML for
`type="html"`).
```suggestion
{{- $content := replace .Content "]]>" "]]]]><![CDATA[>" -}}
<content type="html">{{ printf "<![CDATA[%s]]>" $content | safeHTML
}}</content>
```
##########
landing-pages/site/layouts/blog/list.xml:
##########
@@ -15,29 +15,29 @@
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-*/}}
-
-<?xml version="1.0" encoding="utf-8"?>
+*/ -}}
+{{- $base := .Site.Params.canonicalBaseURL | default "" -}}
+{{ printf "<?xml version=\"1.0\" encoding=\"utf-8\"?>" | safeHTML }}
<feed xmlns="http://www.w3.org/2005/Atom";>
<title>{{ .Site.Title }} Blog</title>
- <link href="{{ .Permalink }}" rel="alternate"/>
-<link href="{{ .Permalink }}index.xml" rel="self" type="application/atom+xml"/>
-<id>{{ .Permalink }}</id>
+ <link href="{{ $base }}{{ .Permalink }}" rel="alternate"/>
+ <link href="{{ $base }}{{ .Permalink }}index.xml" rel="self"
type="application/atom+xml"/>
+ <id>{{ $base }}{{ .Permalink }}</id>
Review Comment:
Building absolute URLs via `{{ $base }}{{ .Permalink }}` assumes
`.Permalink` is a path. If `baseURL` is ever set to an absolute URL (e.g. via
CLI/env in other build contexts), `.Permalink` becomes absolute and this
concatenation would produce an invalid URL (double scheme/host). Consider using
`.RelPermalink` (path-only) and joining with `canonicalBaseURL` (e.g. via
`urls.JoinPath` or trimming trailing/leading slashes) for the `<link>`/`<id>`
values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
