Merge "WebLinks: Clarify that file path must be provided without leading slash"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 9ace97d3..139d6f8 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -535,6 +535,13 @@
+
By default, `false`.
+[[auth.cookieHttpOnly]]auth.cookieHttpOnly::
++
+Sets "httpOnly" flag of the authentication cookie. If `true`, cookie
+values can't be accessed by client side scripts.
++
+By default, `false`.
+
[[auth.emailFormat]]auth.emailFormat::
+
Optional format string to construct user email addresses out of
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index c6e5623..4e60a30 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6985,11 +6985,11 @@
|=================================
|Field Name ||Description
|`patch` |required|
-|`allow_conflicts` |optional|
-If true, tolerate conflicts and add conflict markers where required.
The patch to be applied. Must be compatible with `git diff` output.
For example, link:#get-patch[Get Patch] output.
The patch must be provided as UTF-8 text, either directly or base64-encoded.
+|`allow_conflicts` |optional|
+If true, tolerate conflicts and add conflict markers where required.
|=================================
[[applypatchpatchset-input]]
diff --git a/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index 9625039..a92d4f0 100644
--- a/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -265,10 +265,10 @@
outCookie.setDomain(domain);
}
- outCookie.setSecure(isSecure(request));
outCookie.setPath(path);
outCookie.setMaxAge(ageSeconds);
- outCookie.setSecure(authConfig.getCookieSecure());
+ outCookie.setSecure(authConfig.getCookieSecure() && isSecure(request));
+ outCookie.setHttpOnly(authConfig.getCookieHttpOnly());
response.addCookie(outCookie);
}
diff --git a/java/com/google/gerrit/httpd/XsrfCookieFilter.java b/java/com/google/gerrit/httpd/XsrfCookieFilter.java
index 079efa4..1ec2649 100644
--- a/java/com/google/gerrit/httpd/XsrfCookieFilter.java
+++ b/java/com/google/gerrit/httpd/XsrfCookieFilter.java
@@ -65,6 +65,7 @@
Cookie c = new Cookie(XsrfConstants.XSRF_COOKIE_NAME, nullToEmpty(v));
c.setPath("/");
c.setSecure(authConfig.getCookieSecure() && isSecure(req));
+ c.setHttpOnly(authConfig.getCookieHttpOnly());
c.setMaxAge(
v != null
? -1 // Set the cookie for this browser session.
diff --git a/java/com/google/gerrit/server/config/AuthConfig.java b/java/com/google/gerrit/server/config/AuthConfig.java
index b6ffcee..e180428 100644
--- a/java/com/google/gerrit/server/config/AuthConfig.java
+++ b/java/com/google/gerrit/server/config/AuthConfig.java
@@ -62,6 +62,7 @@
private final String cookiePath;
private final String cookieDomain;
private final boolean cookieSecure;
+ private final boolean cookieHttpOnly;
private final SignedToken emailReg;
private final boolean allowRegisterNewEmail;
private final boolean userNameCaseInsensitive;
@@ -91,6 +92,7 @@
cookiePath = cfg.getString("auth", null, "cookiepath");
cookieDomain = cfg.getString("auth", null, "cookiedomain");
cookieSecure = cfg.getBoolean("auth", "cookiesecure", false);
+ cookieHttpOnly = cfg.getBoolean("auth", "cookiehttponly", false);
trustContainerAuth = cfg.getBoolean("auth", "trustContainerAuth", false);
enableRunAs = cfg.getBoolean("auth", null, "enableRunAs", true);
gitBasicAuthPolicy = getBasicAuthPolicy(cfg);
@@ -218,6 +220,10 @@
return cookieSecure;
}
+ public boolean getCookieHttpOnly() {
+ return cookieHttpOnly;
+ }
+
public SignedToken getEmailRegistrationToken() {
return emailReg;
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-action.ts b/polygerrit-ui/app/elements/checks/gr-checks-action.ts
index 51f1629..abd8a23 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-action.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-action.ts
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {LitElement, PropertyValues, css, html} from 'lit';
-import {customElement, property} from 'lit/decorators.js';
+import {customElement, property, state} from 'lit/decorators.js';
import {Action, NOT_USEFUL, USEFUL} from '../../api/checks';
import {assertIsDefined} from '../../utils/common-util';
import {resolve} from '../../models/dependency';
@@ -24,6 +24,9 @@
@property({type: String, reflect: true})
icon?: string;
+ @state()
+ clicked = false;
+
private getChecksModel = resolve(this, checksModelToken);
override connectedCallback() {
@@ -81,7 +84,9 @@
private renderName() {
if (!this.icon) return html`${this.action.name}`;
- return html` <gr-icon icon=${this.icon}></gr-icon> `;
+ return html`
+ <gr-icon ?filled=${this.clicked} icon=${this.icon}></gr-icon>
+ `;
}
private renderTooltip() {
@@ -94,7 +99,13 @@
}
handleClick(e: Event) {
- e.stopPropagation();
+ if (this.action.name === USEFUL || this.action.name === NOT_USEFUL) {
+ this.clicked = true;
+ } else {
+ // For useful clicks the parent wants to receive the click for changing
+ // the "Was this helpful?" label.
+ e.stopPropagation();
+ }
this.getChecksModel().triggerAction(this.action, undefined, this.context);
}
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 8958deb..1752da0 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -645,6 +645,9 @@
@property({type: Boolean})
hideCodePointers = false;
+ @state()
+ notUsefulLabel = 'Was this helpful?';
+
private getChangeModel = resolve(this, changeModelToken);
private readonly getViewModel = resolve(this, changeViewModelToken);
@@ -785,9 +788,15 @@
if (!useful || !notUseful) return;
return html`
<div class="useful">
- <div class="title">Was this helpful?</div>
- <gr-checks-action .action=${useful}></gr-checks-action>
- <gr-checks-action .action=${notUseful}></gr-checks-action>
+ <div class="title">${this.notUsefulLabel}</div>
+ <gr-checks-action
+ @click=${() => (this.notUsefulLabel = 'Thanks!')}
+ .action=${useful}
+ ></gr-checks-action>
+ <gr-checks-action
+ @click=${() => (this.notUsefulLabel = 'Sorry about that')}
+ .action=${notUseful}
+ ></gr-checks-action>
</div>
`;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index b57e4ff..d183211 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -126,7 +126,7 @@
this.repoCommentLinks = repoCommentLinks;
// Always linkify URLs starting with https?://
this.repoCommentLinks['ALWAYS_LINK_HTTP'] = {
- match: '(https?://((?!&(gt|lt|amp|quot|apos);)\\S)+[\\w/~-])',
+ match: '(https?://((?!&(gt|lt|quot|apos);)\\S)+[\\w/~-])',
link: '$1',
enabled: true,
};
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index 23f1594..c1b38d5 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -235,6 +235,11 @@
await checkLinking('https://www.google.com/');
await checkLinking('https://www.google.com/asdf~');
await checkLinking('https://www.google.com/asdf-');
+ await checkLinking('https://www.google.com/asdf-');
+ // matches & part as well, even we first linkify and then htmlEscape
+ await checkLinking(
+ 'https://google.com/traces/list?project=gerrit&tid=123'
+ );
});
});
@@ -710,6 +715,10 @@
await checkLinking('http://www.google.com');
await checkLinking('https://www.google.com');
await checkLinking('https://www.google.com/');
+ // matches & part as well, even we first linkify and then htmlEscape
+ await checkLinking(
+ 'https://google.com/traces/list?project=gerrit&tid=123'
+ );
});
suite('user suggest fix', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
index e9c132a..d27bd2d 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
@@ -9,22 +9,16 @@
import {PluginApi} from '../../../api/plugin';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
-import {readJSONResponsePayload} from '../gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {
+ readJSONResponsePayload,
+ throwingErrorCallback,
+} from '../gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
async function getErrorMessage(response: Response): Promise<string> {
const text = await response.text();
return text || `${response.status}`;
}
-// This is an internal error, that must never be visible outside of this
-// file. It is used only inside GrPluginRestApi.send method. See detailed
-// explanation in the GrPluginRestApi.send method.
-class ResponseError extends Error {
- public constructor(readonly response: Response) {
- super();
- }
-}
-
export class GrPluginRestApi implements RestPluginApi {
constructor(
private readonly restApi: RestApiService,
@@ -120,53 +114,31 @@
contentType?: string
) {
this.reporting.trackApi(this.plugin, 'rest', 'send');
- // Plugins typically don't want Gerrit to show error dialogs for failed
- // requests. So we are defining a default errFn here, even if it is not
- // explicitly set by the caller.
- // TODO: We are soon getting rid of the `errFn` altogether. There are only
- // 2 known usages of errFn in plugins: delete-project and verify-status.
- errFn =
- errFn ??
- ((response: Response | null | undefined, error?: Error) => {
- if (error) throw error;
- // Some plugins show an error message if send is failed, smth like:
- // pluginApi.send(...).catch(err => showError(err));
- // The response can contain an error text, but getting this text is
- // an asynchronous operation. At the same time, the errFn must be a
- // synchronous function.
- // As a workaround, we throw an ResponseError here and then catch
- // it inside a catch block below and read the message.
- if (response) throw new ResponseError(response);
- throw new Error('Generic REST API error.');
- });
- return this.fetch(method, url, payload, errFn, contentType)
- .then(response => {
- // Will typically not happen. The response can only be unset, if the
- // errFn handles the error and then returns void or undefined or null.
- // But the errFn above always throws.
- if (!response) {
- throw new Error('plugin rest-api call failed');
- }
- // Will typically not happen. errFn will have dealt with that and the
- // caller will get a rejected promise already.
- if (response.status < 200 || response.status >= 300) {
- return getErrorMessage(response).then(msg =>
- Promise.reject(new Error(msg))
- );
- } else {
- return readJSONResponsePayload(response).then(
- obj => obj.parsed
- ) as Promise<T>;
- }
- })
- .catch(err => {
- if (err instanceof ResponseError) {
- return getErrorMessage(err.response).then(msg => {
- throw new Error(msg);
- });
- }
- throw err;
- });
+ return this.fetch(
+ method,
+ url,
+ payload,
+ errFn ?? throwingErrorCallback,
+ contentType
+ ).then(response => {
+ // Will typically not happen. The response can only be unset, if the
+ // errFn handles the error and then returns void or undefined or null.
+ // But the errFn above always throws.
+ if (!response) {
+ throw new Error('plugin rest-api call failed');
+ }
+ // Will typically not happen. errFn will have dealt with that and the
+ // caller will get a rejected promise already.
+ if (response.status < 200 || response.status >= 300) {
+ return getErrorMessage(response).then(msg =>
+ Promise.reject(new Error(msg))
+ );
+ } else {
+ return readJSONResponsePayload(response).then(
+ obj => obj.parsed
+ ) as Promise<T>;
+ }
+ });
}
get<T>(url: string) {
diff --git a/polygerrit-ui/app/embed/gr-textarea.ts b/polygerrit-ui/app/embed/gr-textarea.ts
index 05bef49..27948fd 100644
--- a/polygerrit-ui/app/embed/gr-textarea.ts
+++ b/polygerrit-ui/app/embed/gr-textarea.ts
@@ -173,6 +173,8 @@
private focused = false;
+ private currentCursorPosition = -1;
+
private readonly isPlaintextOnlySupported = supportsPlainTextEditing();
static override get styles() {
@@ -488,6 +490,19 @@
event.preventDefault();
this.fire('saveShortcut');
}
+ // Prevent looping of cursor position when CTRL+ARROW_LEFT/ARROW_RIGHT is
+ // pressed.
+ if (event.ctrlKey || event.metaKey || event.altKey) {
+ if (event.key === 'ArrowLeft' && this.currentCursorPosition === 0) {
+ event.preventDefault();
+ }
+ if (
+ event.key === 'ArrowRight' &&
+ this.currentCursorPosition === (this.value?.length ?? 0)
+ ) {
+ event.preventDefault();
+ }
+ }
await this.toggleHintVisibilityIfAny();
}
@@ -597,7 +612,9 @@
}
private onCursorPositionChange() {
- this.fire('cursorPositionChange', {position: this.getCursorPosition()});
+ const cursorPosition = this.getCursorPosition();
+ this.fire('cursorPositionChange', {position: cursorPosition});
+ this.currentCursorPosition = cursorPosition;
}
private async updateValueInDom() {
diff --git a/polygerrit-ui/app/embed/gr-textarea_test.ts b/polygerrit-ui/app/embed/gr-textarea_test.ts
index b701dcb..d125d2f 100644
--- a/polygerrit-ui/app/embed/gr-textarea_test.ts
+++ b/polygerrit-ui/app/embed/gr-textarea_test.ts
@@ -232,4 +232,56 @@
assert.equal(element.value, oldValue + hint);
});
+
+ test('when cursor is at end, Mod + ArrowRight does not change cursor position', async () => {
+ const CURSOR_POSITION_CHANGE_EVENT = 'cursorPositionChange';
+ let cursorPosition = -1;
+ const value = 'Hola amigos';
+ const editableDiv = element.shadowRoot!.querySelector(
+ '.editableDiv'
+ ) as HTMLDivElement;
+ element.addEventListener(CURSOR_POSITION_CHANGE_EVENT, (event: Event) => {
+ const detail = (event as CustomEvent<CursorPositionChangeEventDetail>)
+ .detail;
+ cursorPosition = detail.position;
+ });
+ await element.updateComplete;
+ element.value = value;
+ await element.putCursorAtEnd();
+ await element.updateComplete;
+
+ editableDiv.dispatchEvent(
+ new KeyboardEvent('keydown', {key: 'ArrowRight', metaKey: true})
+ );
+ await element.updateComplete;
+ await rafPromise();
+
+ assert.equal(cursorPosition, value.length);
+ });
+
+ test('when cursor is at 0, Mod + ArrowLeft does not change cursor position', async () => {
+ const CURSOR_POSITION_CHANGE_EVENT = 'cursorPositionChange';
+ let cursorPosition = -1;
+ const value = 'Hola amigos';
+ const editableDiv = element.shadowRoot!.querySelector(
+ '.editableDiv'
+ ) as HTMLDivElement;
+ element.addEventListener(CURSOR_POSITION_CHANGE_EVENT, (event: Event) => {
+ const detail = (event as CustomEvent<CursorPositionChangeEventDetail>)
+ .detail;
+ cursorPosition = detail.position;
+ });
+ await element.updateComplete;
+ element.value = value;
+ element.setCursorPosition(0);
+ await element.updateComplete;
+
+ editableDiv.dispatchEvent(
+ new KeyboardEvent('keydown', {key: 'ArrowLeft', metaKey: true})
+ );
+ await element.updateComplete;
+ await rafPromise();
+
+ assert.equal(cursorPosition, 0);
+ });
});
diff --git a/polygerrit-ui/app/models/checks/checks-fakes.ts b/polygerrit-ui/app/models/checks/checks-fakes.ts
index be11278..f6fe242 100644
--- a/polygerrit-ui/app/models/checks/checks-fakes.ts
+++ b/polygerrit-ui/app/models/checks/checks-fakes.ts
@@ -5,6 +5,7 @@
*/
import {
Action,
+ ActionResult,
Category,
Link,
LinkIcon,
@@ -185,13 +186,25 @@
actions: [
{
name: 'useful',
+ tooltip: 'This check result was helpful',
callback: () =>
- Promise.resolve({message: 'fake "useful report" triggered'}),
+ new Promise(resolve => {
+ setTimeout(
+ () => resolve({message: 'Feedback recorded.'} as ActionResult),
+ 1000
+ );
+ }),
},
{
name: 'not-useful',
+ tooltip: 'This check result was not helpful',
callback: () =>
- Promise.resolve({message: 'fake "not useful report" triggered'}),
+ new Promise(resolve => {
+ setTimeout(
+ () => resolve({message: 'Feedback recorded.'} as ActionResult),
+ 1000
+ );
+ }),
},
],
fixes: [
