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

WIP/POC: crossword page in DCR #12511

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions dotcom-rendering/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
type GuardianCrossword = import('mycrossword').GuardianCrossword;

type SharePlatform =
| 'facebook'
| 'twitter'
Expand Down Expand Up @@ -188,6 +190,12 @@ declare module '*.svg' {
export default content;
}

declare module '!to-string-loader*' {
const value: any;
// eslint-disable-next-line import/no-default-export -- This is how we import mycrossword's CSS bundle
export default value;
}

// Extend PerformanceEntry from lib.dom.ts with current 'In Draft' properties (to allow access as use in browsers that support)
// lib.dom.ts: https://microsoft.github.io/PowerBI-JavaScript/interfaces/_node_modules_typedoc_node_modules_typescript_lib_lib_dom_d_.performanceentry.html
// Draft: https://wicg.github.io/element-timing/#sec-performance-element-timing
Expand Down
1 change: 1 addition & 0 deletions dotcom-rendering/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
"lz-string": "1.5.0",
"mockdate": "3.0.5",
"node-fetch": "3.3.2",
"mycrossword": "1.2.4",
"postcss-styled-syntax": "0.6.3",
"preact": "10.15.1",
"preact-render-to-string": "6.0.2",
Expand Down
3 changes: 3 additions & 0 deletions dotcom-rendering/src/components/ArticleContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const articleWidth = (format: ArticleFormat) => {
}
`;
}
case ArticleDesign.Crossword:
/* The crossword player manages its own width; */
return null;
case ArticleDesign.Video:
case ArticleDesign.Audio:
return css`
Expand Down
5 changes: 5 additions & 0 deletions dotcom-rendering/src/components/BylineLink.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ArticleDesign, type ArticleFormat, isString } from '@guardian/libs';
import { Hide } from '@guardian/source/react-components';
import { DottedLines } from '@guardian/source-development-kitchen/react-components';
import { Fragment } from 'react';
import {
getBylineComponentsFromTokens,
getSoleContributor,
Expand Down Expand Up @@ -159,6 +160,10 @@ const getRenderedTokens = (
);
}

if (design === ArticleDesign.Crossword && renderedTokens.length > 0) {
return [(<Fragment key="set-by">Set by: </Fragment>), ...renderedTokens];
}

return renderedTokens;
};

Expand Down
15 changes: 15 additions & 0 deletions dotcom-rendering/src/components/Crossword.importable.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { GuardianCrossword } from 'mycrossword';
import { MyCrossword } from 'mycrossword/dist/cjs/components';
import xwCss from '!to-string-loader!css-loader!mycrossword/dist/index.css';

Check failure on line 3 in dotcom-rendering/src/components/Crossword.importable.tsx

View workflow job for this annotation

GitHub Actions / lint / check

Unable to resolve path to module '!to-string-loader!css-loader!mycrossword/dist/index.css'

export type CrosswordProps = {
id: string;
crossword: GuardianCrossword;
};

export const Crossword = ({ crossword, id }: CrosswordProps) => (
<>
<style>{xwCss}</style>
<MyCrossword id={id} data={crossword} />
</>
);
26 changes: 26 additions & 0 deletions dotcom-rendering/src/components/CrosswordInstructions.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { css } from '@emotion/react';
import {
textEgyptian17,
textEgyptianBold17,
} from '@guardian/source/foundations';

const instructionsStyles = css`
${textEgyptian17};
white-space: pre-line;
`;
const headerStyles = css`
${textEgyptianBold17};
white-space: pre-line;
`;
export const CrosswordInstructions = ({
instructions,
className,
}: {
instructions: string;
className?: string;
}) => (
<div css={instructionsStyles} className={className}>
<strong css={headerStyles}>Special instructions: </strong>
{instructions}
</div>
);
49 changes: 49 additions & 0 deletions dotcom-rendering/src/components/CrosswordLinks.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { css } from '@emotion/react';
import { textSans15 } from '@guardian/source/foundations';
import { palette } from '../palette';

const crosswordLinkStyles = css`
${textSans15};

a {
color: ${palette('--standfirst-link-text')};
text-decoration: none;
:hover {
border-bottom: 1px solid ${palette('--standfirst-link-border')};
}
}
`;

export const CrosswordLinks = ({
crossword,
className,
}: {
crossword: GuardianCrossword;
className?: string;
}) => {
return (
<span css={crosswordLinkStyles} className={className}>
<a
target="_blank"
href={`/crosswords/${crossword.crosswordType}/${crossword.number}/print`}
rel="noreferrer"
>
Print
</a>{' '}
|{' '}
{!!crossword.pdf && (
<>
<a target="_blank" href={crossword.pdf} rel="noreferrer">

Check failure

Code scanning / CodeQL

Client-side cross-site scripting High

Cross-site scripting vulnerability due to
user-provided value
.
Cross-site scripting vulnerability due to
user-provided value
.
Cross-site scripting vulnerability due to
user-provided value
.
Cross-site scripting vulnerability due to
user-provided value
.

Copilot Autofix AI 20 days ago

To fix the problem, we need to ensure that the crossword.pdf value is properly sanitized before being used in the href attribute. This can be achieved by using a library like DOMPurify to sanitize the URL.

  • Import the DOMPurify library.
  • Use DOMPurify.sanitize to clean the crossword.pdf value before using it in the href attribute.
Suggested changeset 1
dotcom-rendering/src/components/CrosswordLinks.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dotcom-rendering/src/components/CrosswordLinks.tsx b/dotcom-rendering/src/components/CrosswordLinks.tsx
--- a/dotcom-rendering/src/components/CrosswordLinks.tsx
+++ b/dotcom-rendering/src/components/CrosswordLinks.tsx
@@ -3,2 +3,3 @@
 import { palette } from '../palette';
+import DOMPurify from 'dompurify';
 
@@ -23,2 +24,3 @@
 }) => {
+	const sanitizedPdf = crossword.pdf ? DOMPurify.sanitize(crossword.pdf) : null;
 	return (
@@ -33,5 +35,5 @@
 			|{' '}
-			{!!crossword.pdf && (
+			{!!sanitizedPdf && (
 				<>
-					<a target="_blank" href={crossword.pdf} rel="noreferrer">
+					<a target="_blank" href={sanitizedPdf} rel="noreferrer">
 						PDF version
EOF
@@ -3,2 +3,3 @@
import { palette } from '../palette';
import DOMPurify from 'dompurify';

@@ -23,2 +24,3 @@
}) => {
const sanitizedPdf = crossword.pdf ? DOMPurify.sanitize(crossword.pdf) : null;
return (
@@ -33,5 +35,5 @@
|{' '}
{!!crossword.pdf && (
{!!sanitizedPdf && (
<>
<a target="_blank" href={crossword.pdf} rel="noreferrer">
<a target="_blank" href={sanitizedPdf} rel="noreferrer">
PDF version
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options

Check warning

Code scanning / CodeQL

Client-side URL redirect Medium

Untrusted URL redirection depends on a
user-provided value
.
Untrusted URL redirection depends on a
user-provided value
.
Untrusted URL redirection depends on a
user-provided value
.
Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix AI 20 days ago

To fix the problem, we need to ensure that the URL in crossword.pdf is validated against a list of trusted URLs or domains before it is used in the href attribute. This can be achieved by creating a function that checks if the URL belongs to a trusted domain and only then allows it to be used.

  1. Create a function to validate the URL.
  2. Use this function to check crossword.pdf before setting it in the href attribute.
Suggested changeset 1
dotcom-rendering/src/components/CrosswordLinks.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dotcom-rendering/src/components/CrosswordLinks.tsx b/dotcom-rendering/src/components/CrosswordLinks.tsx
--- a/dotcom-rendering/src/components/CrosswordLinks.tsx
+++ b/dotcom-rendering/src/components/CrosswordLinks.tsx
@@ -16,2 +16,12 @@
 
+const isValidUrl = (url: string) => {
+	try {
+		const trustedDomains = ['theguardian.com', 'guim.co.uk'];
+		const parsedUrl = new URL(url);
+		return trustedDomains.includes(parsedUrl.hostname);
+	} catch (e) {
+		return false;
+	}
+};
+
 export const CrosswordLinks = ({
@@ -33,3 +43,3 @@
 			|{' '}
-			{!!crossword.pdf && (
+			{!!crossword.pdf && isValidUrl(crossword.pdf) && (
 				<>
EOF
@@ -16,2 +16,12 @@

const isValidUrl = (url: string) => {
try {
const trustedDomains = ['theguardian.com', 'guim.co.uk'];
const parsedUrl = new URL(url);
return trustedDomains.includes(parsedUrl.hostname);
} catch (e) {
return false;
}
};

export const CrosswordLinks = ({
@@ -33,3 +43,3 @@
|{' '}
{!!crossword.pdf && (
{!!crossword.pdf && isValidUrl(crossword.pdf) && (
<>
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
PDF version
</a>{' '}
|{' '}
</>
)}
<a
href={`/crosswords/accessible/${crossword.crosswordType}/${crossword.number}`}
>
Accessible version
</a>
</span>
);
};
Loading
Loading