Skip to content

refactor: v2 release #6903

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

Open
wants to merge 1,041 commits into
base: main
Choose a base branch
from
Open

refactor: v2 release #6903

wants to merge 1,041 commits into from

Conversation

wmertens
Copy link
Member

@wmertens wmertens commented Sep 22, 2024

This PR is for showing progress on v2, and having installable npm packages.

DO NOT MERGE

The changes are meant to be readable and maintainable, so if things are unclear please let us know.

Copy link

changeset-bot bot commented Sep 22, 2024

🦋 Changeset detected

Latest commit: 495e8d9

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Sep 23, 2024

Open in StackBlitz

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@6903
npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@6903
npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@6903
npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@6903

commit: 495e8d9

Copy link
Contributor

github-actions bot commented Sep 23, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview 495e8d9

const insertBefore = journal[idx++] as Element | Text | null;
let newChild: any;
while (idx < length && typeof (newChild = journal[idx]) !== 'number') {
insertParent.insertBefore(newChild, insertBefore);

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium

DOM text
is reinterpreted as HTML without escaping meta-characters.
DOM text
is reinterpreted as HTML without escaping meta-characters.
@wmertens wmertens changed the title refactor: v2 framework rewrite refactor: v2 release Oct 8, 2024
@wmertens wmertens marked this pull request as ready for review October 17, 2024 21:25
@wmertens wmertens requested review from a team as code owners October 17, 2024 21:25
c.push(`\n/** Qwik Router Entries (${entries.length}) */`);
for (let i = 0; i < entries.length; i++) {
const entry = entries[i];
c.push(`export const ${entry.id} = () => import(${JSON.stringify(entry.filePath)});`);

Check warning

Code scanning / CodeQL

Improper code sanitization Medium

Code construction depends on an
improperly sanitized value
.

Copilot Autofix

AI 6 months ago

To fix the problem, we need to ensure that entry.filePath is properly sanitized before being used in the dynamically generated JavaScript code. We can achieve this by escaping potentially dangerous characters in the entry.filePath string. This can be done by implementing a function similar to escapeUnsafeChars from the example provided in the background section.

  1. Implement a function escapeUnsafeChars to escape potentially dangerous characters.
  2. Use this function to sanitize entry.filePath before including it in the generated code.
Suggested changeset 1
packages/qwik-router/src/buildtime/runtime-generation/generate-entries.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/qwik-router/src/buildtime/runtime-generation/generate-entries.ts b/packages/qwik-router/src/buildtime/runtime-generation/generate-entries.ts
--- a/packages/qwik-router/src/buildtime/runtime-generation/generate-entries.ts
+++ b/packages/qwik-router/src/buildtime/runtime-generation/generate-entries.ts
@@ -2,2 +2,20 @@
 
+function escapeUnsafeChars(str: string): string {
+  const charMap: { [key: string]: string } = {
+    '<': '\\u003C',
+    '>': '\\u003E',
+    '/': '\\u002F',
+    '\\': '\\\\',
+    '\b': '\\b',
+   '\f': '\\f',
+   '\n': '\\n',
+   '\r': '\\r',
+   '\t': '\\t',
+   '\0': '\\0',
+   '\u2028': '\\u2028',
+   '\u2029': '\\u2029'
+ };
+ return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x]);
+}
+
 export function createEntries(ctx: BuildContext, c: string[]) {
@@ -25,3 +43,3 @@
     const entry = entries[i];
-    c.push(`export const ${entry.id} = () => import(${JSON.stringify(entry.filePath)});`);
+    c.push(`export const ${entry.id} = () => import(${escapeUnsafeChars(JSON.stringify(entry.filePath))});`);
   }
EOF
@@ -2,2 +2,20 @@

function escapeUnsafeChars(str: string): string {
const charMap: { [key: string]: string } = {
'<': '\\u003C',
'>': '\\u003E',
'/': '\\u002F',
'\\': '\\\\',
'\b': '\\b',
'\f': '\\f',
'\n': '\\n',
'\r': '\\r',
'\t': '\\t',
'\0': '\\0',
'\u2028': '\\u2028',
'\u2029': '\\u2029'
};
return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x]);
}

export function createEntries(ctx: BuildContext, c: string[]) {
@@ -25,3 +43,3 @@
const entry = entries[i];
c.push(`export const ${entry.id} = () => import(${JSON.stringify(entry.filePath)});`);
c.push(`export const ${entry.id} = () => import(${escapeUnsafeChars(JSON.stringify(entry.filePath))});`);
}
Copilot is powered by AI and may make mistakes. Always verify output.
}

if (key === dangerouslySetInnerHTML) {
element.innerHTML = value as string;

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium

DOM text
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix

AI 13 days ago

To fix the issue, we need to ensure that the value assigned to dangerouslySetInnerHTML is properly sanitized or escaped to prevent XSS vulnerabilities. The best approach is to use a utility function like escapeHTML to encode special HTML characters (<, >, &, etc.) in the value before assigning it to innerHTML. This ensures that any potentially malicious content is rendered as plain text rather than executable HTML.

Changes will be made in the vnode_diff function in packages/qwik/src/core/client/vnode-diff.ts to apply escapeHTML to value before assigning it to element.innerHTML.


Suggested changeset 1
packages/qwik/src/core/client/vnode-diff.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/qwik/src/core/client/vnode-diff.ts b/packages/qwik/src/core/client/vnode-diff.ts
--- a/packages/qwik/src/core/client/vnode-diff.ts
+++ b/packages/qwik/src/core/client/vnode-diff.ts
@@ -655,3 +655,3 @@
         if (key === dangerouslySetInnerHTML) {
-          element.innerHTML = value as string;
+          element.innerHTML = escapeHTML(value as string);
           element.setAttribute(QContainerAttr, QContainerValue.HTML);
EOF
@@ -655,3 +655,3 @@
if (key === dangerouslySetInnerHTML) {
element.innerHTML = value as string;
element.innerHTML = escapeHTML(value as string);
element.setAttribute(QContainerAttr, QContainerValue.HTML);
Copilot is powered by AI and may make mistakes. Always verify output.
wmertens and others added 30 commits May 30, 2025 16:28
…PIs, show an explanation (#7571)

* fix(repl): improve deepUpdate logic for array item matching

* fix  it

* fix(core): prevent server-side usage of browser APIs

Add error handling for browser API usage during server-side rendering. The changes include:
- Adding a new error code `QError.notUsingBrowserAPiInserver` to handle cases where browser APIs are used on the server.
- Updating the `use-task.spec.tsx` test to catch and verify the new error.
- Modifying `qrl-class.ts` to throw the error when browser APIs are accessed during server-side rendering.

This ensures that developers are aware of and avoid using browser-specific APIs in server-side contexts, preventing runtime errors.

* add change

* fix(qrl): improve error message for browser API usage in SSR

* docs(changeset): update changeset from major to patch

Update the changeset to correctly reflect the version bump as a patch instead of a major version change. This ensures accurate versioning and release notes.

* feat(core): improve error message for SSR browser API usage

When an error occurs during SSR due to using browser APIs, provide a clearer explanation to help developers understand the issue and how to fix it. This enhances debugging experience and reduces confusion.

* test(use-task): update test case for error handling in useTask$

Ensure the test correctly verifies error messages thrown within useTask$ by adding a new component and refining assertions

* test(use-task): update error handling test to use ErrorProvider

Modify the test to wrap components with ErrorProvider and update the expected error message. This ensures consistent error handling and improves test reliability.

* feat(eslint-plugin-qwik): add scope-use-task rule to enforce server guard in useTask$

The new rule `scope-use-task` ensures that Node.js APIs (e.g., `process.env`) are only used within `useTask$` when guarded by a server check (e.g., `isServer`). This prevents unsafe usage of Node.js APIs in client-side code. The rule also checks for indirect API usage via function calls and provides appropriate error messages to guide developers in adding the necessary guards.

* test(scope-use-task): update test cases for unsafe API usage

Refactor test files to include additional scenarios for unsafe API usage in Qwik components. This ensures comprehensive coverage for detecting unsafe API calls in various contexts.

* fix(qrl): handle window reference errors in server mode

Add try-catch block to handle ReferenceError for 'window' in server mode during development. Improve error message to guide developers to use 'if (isBrowser)' checks when needed.

Remove duplicate error handling code that was previously in the invoke function since it's now handled at the qrl level.

* ❤️
caused repl segments to no longer show
chore: merge main into v2: qwikloader in a separate bundle + fixups
this was hiding output when warnings
fix(repl): always update even when diagnostics
or you get some circular warnings
fix(core): import isDev from build directly
fix(manifest): order qwikloader key higher up in the json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants