Add test for "fetch redirect", add CSS value validation for external render (#37207)
By the way, fix the checkAppUrl message for #37212
This commit is contained in:
@@ -18,9 +18,10 @@ func FetchRedirectDelegate(resp http.ResponseWriter, req *http.Request) {
|
|||||||
// then frontend needs this delegate to redirect to the new location with hash correctly.
|
// then frontend needs this delegate to redirect to the new location with hash correctly.
|
||||||
redirect := req.FormValue("redirect")
|
redirect := req.FormValue("redirect")
|
||||||
if req.Method != http.MethodPost || !httplib.IsCurrentGiteaSiteURL(req.Context(), redirect) {
|
if req.Method != http.MethodPost || !httplib.IsCurrentGiteaSiteURL(req.Context(), redirect) {
|
||||||
resp.WriteHeader(http.StatusBadRequest)
|
http.Error(resp, "Bad Request", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
resp.Header().Add("Location", redirect)
|
// no OpenRedirect, the "redirect" is validated by "IsCurrentGiteaSiteURL" above
|
||||||
|
resp.Header().Set("Location", redirect)
|
||||||
resp.WriteHeader(http.StatusSeeOther)
|
resp.WriteHeader(http.StatusSeeOther)
|
||||||
}
|
}
|
||||||
|
|||||||
48
routers/common/redirect_test.go
Normal file
48
routers/common/redirect_test.go
Normal file
@@ -0,0 +1,48 @@
|
|||||||
|
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
|
package common
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/modules/setting"
|
||||||
|
"code.gitea.io/gitea/modules/test"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFetchRedirectDelegate(t *testing.T) {
|
||||||
|
defer test.MockVariableValue(&setting.AppURL, "https://gitea/")()
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
method string
|
||||||
|
input string
|
||||||
|
status int
|
||||||
|
}{
|
||||||
|
{method: "POST", input: "/foo?k=v", status: http.StatusSeeOther},
|
||||||
|
{method: "GET", input: "/foo?k=v", status: http.StatusBadRequest},
|
||||||
|
{method: "POST", input: `\/foo?k=v`, status: http.StatusBadRequest},
|
||||||
|
{method: "POST", input: `\\/foo?k=v`, status: http.StatusBadRequest},
|
||||||
|
{method: "POST", input: "https://gitea/xxx", status: http.StatusSeeOther},
|
||||||
|
{method: "POST", input: "https://other/xxx", status: http.StatusBadRequest},
|
||||||
|
}
|
||||||
|
for _, c := range cases {
|
||||||
|
t.Run(c.method+" "+c.input, func(t *testing.T) {
|
||||||
|
resp := httptest.NewRecorder()
|
||||||
|
req := httptest.NewRequest(c.method, "/?redirect="+url.QueryEscape(c.input), nil)
|
||||||
|
FetchRedirectDelegate(resp, req)
|
||||||
|
assert.Equal(t, c.status, resp.Code)
|
||||||
|
if c.status == http.StatusSeeOther {
|
||||||
|
assert.Equal(t, c.input, resp.Header().Get("Location"))
|
||||||
|
} else {
|
||||||
|
assert.Empty(t, resp.Header().Get("Location"))
|
||||||
|
assert.Equal(t, "Bad Request", strings.TrimSpace(resp.Body.String()))
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
25
web_src/js/external-render-helper.test.ts
Normal file
25
web_src/js/external-render-helper.test.ts
Normal file
@@ -0,0 +1,25 @@
|
|||||||
|
import './external-render-helper.ts';
|
||||||
|
|
||||||
|
test('isValidCssColor', async () => {
|
||||||
|
const isValidCssColor = window.testModules.externalRenderHelper!.isValidCssColor;
|
||||||
|
expect(isValidCssColor(null)).toBe(false);
|
||||||
|
expect(isValidCssColor('')).toBe(false);
|
||||||
|
|
||||||
|
expect(isValidCssColor('#123')).toBe(true);
|
||||||
|
expect(isValidCssColor('#1234')).toBe(true);
|
||||||
|
expect(isValidCssColor('#abcabc')).toBe(true);
|
||||||
|
expect(isValidCssColor('#abcabc12')).toBe(true);
|
||||||
|
|
||||||
|
expect(isValidCssColor('rgb(255 255 255)')).toBe(true);
|
||||||
|
expect(isValidCssColor('rgb(0, 255, 255)')).toBe(true);
|
||||||
|
|
||||||
|
// examples from MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/rgb
|
||||||
|
expect(isValidCssColor('rgb(255 255 255 / 50%)')).toBe(true);
|
||||||
|
expect(isValidCssColor('rgb(from #123456 hwb(120deg 10% 20%) calc(g + 40) b / 0.5)')).toBe(true);
|
||||||
|
|
||||||
|
expect(isValidCssColor('#123 ; other')).toBe(false);
|
||||||
|
expect(isValidCssColor('#123 : other')).toBe(false);
|
||||||
|
expect(isValidCssColor('#rgb(0, 255, 255); other')).toBe(false);
|
||||||
|
expect(isValidCssColor('#rgb(0, 255, 255)} other')).toBe(false);
|
||||||
|
expect(isValidCssColor('url(other)')).toBe(false);
|
||||||
|
});
|
||||||
@@ -15,6 +15,17 @@ RENDER_COMMAND = `echo '<div style="width: 100%; height: 2000px; border: 10px so
|
|||||||
|
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
// Check whether the user-provided color value is a valid CSS color format to avoid CSS injection.
|
||||||
|
// Don't extract this function to a common module, because this file is an IIFE module for external render
|
||||||
|
// and should not have any dependency to avoid potential conflicts.
|
||||||
|
function isValidCssColor(s: string | null): boolean {
|
||||||
|
if (!s) return false;
|
||||||
|
// it should only be in format "#hex" or "rgb(...)", because it comes from a computed style's color value
|
||||||
|
const reHex = /^#([0-9a-fA-F]{3}|[0-9a-fA-F]{4}|[0-9a-fA-F]{6}|[0-9a-fA-F]{8})$/;
|
||||||
|
const reRgb = /^rgb\([^{}'";:]+\)$/;
|
||||||
|
return reHex.test(s) || reRgb.test(s);
|
||||||
|
}
|
||||||
|
|
||||||
const url = new URL(window.location.href);
|
const url = new URL(window.location.href);
|
||||||
|
|
||||||
const isDarkTheme = url.searchParams.get('gitea-is-dark-theme') === 'true';
|
const isDarkTheme = url.searchParams.get('gitea-is-dark-theme') === 'true';
|
||||||
@@ -23,7 +34,7 @@ if (isDarkTheme) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const backgroundColor = url.searchParams.get('gitea-iframe-bgcolor');
|
const backgroundColor = url.searchParams.get('gitea-iframe-bgcolor');
|
||||||
if (backgroundColor) {
|
if (isValidCssColor(backgroundColor)) {
|
||||||
// create a style element to set background color, then it can be overridden by the content page's own style if needed
|
// create a style element to set background color, then it can be overridden by the content page's own style if needed
|
||||||
const style = document.createElement('style');
|
const style = document.createElement('style');
|
||||||
style.textContent = `
|
style.textContent = `
|
||||||
@@ -75,3 +86,7 @@ if (iframeId) {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (window.testModules) {
|
||||||
|
window.testModules.externalRenderHelper = {isValidCssColor};
|
||||||
|
}
|
||||||
|
|||||||
@@ -156,8 +156,8 @@ export function checkAppUrl() {
|
|||||||
if (curUrl.startsWith(appUrl) || `${curUrl}/` === appUrl) {
|
if (curUrl.startsWith(appUrl) || `${curUrl}/` === appUrl) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
showGlobalErrorMessage(`Your ROOT_URL in app.ini is "${appUrl}", it's unlikely matching the site you are visiting.
|
showGlobalErrorMessage(`The detected web site URL is "${appUrl}", it's unlikely matching the site config.
|
||||||
Mismatched ROOT_URL config causes wrong URL links for web UI/mail content/webhook notification/OAuth2 sign-in.`, 'warning');
|
Mismatched app.ini ROOT_URL or reverse proxy "Host/X-Forwarded-Proto" config might cause wrong URL links for web UI/mail content/webhook notification/OAuth2 sign-in.`, 'warning');
|
||||||
}
|
}
|
||||||
|
|
||||||
export function checkAppUrlScheme() {
|
export function checkAppUrlScheme() {
|
||||||
|
|||||||
9
web_src/js/globals.d.ts
vendored
9
web_src/js/globals.d.ts
vendored
@@ -68,6 +68,15 @@ interface Window {
|
|||||||
turnstile: any,
|
turnstile: any,
|
||||||
hcaptcha: any,
|
hcaptcha: any,
|
||||||
|
|
||||||
|
// Make IIFE private functions can be tested in unit tests, without exposing the IIFE module to global scope.
|
||||||
|
// Otherwise, when using "export" in IIFE code, the compiled JS will inject global "var externalRenderHelper = ..."
|
||||||
|
// which is not expected and may cause conflicts with other modules.
|
||||||
|
testModules: {
|
||||||
|
externalRenderHelper?: {
|
||||||
|
isValidCssColor(s: string | null): boolean,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// do not add more properties here unless it is a must
|
// do not add more properties here unless it is a must
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -24,4 +24,6 @@ window.config = {
|
|||||||
i18n: {},
|
i18n: {},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
window.testModules = {};
|
||||||
|
|
||||||
export {}; // mark as module for top-level await
|
export {}; // mark as module for top-level await
|
||||||
|
|||||||
Reference in New Issue
Block a user