Skip to content

Commit

Permalink
Make key URL rules high priority to avoid conflicts
Browse files Browse the repository at this point in the history
Previously this could cause confusion - the Android rule didn't apply if
other rules overrode it, and a passthrough rule could result in
'amiusing' returning false unexpectedly. This makes that work as
expected, and provides a little UI signifier to help make that clearer.
  • Loading branch information
pimterry committed Feb 1, 2024
1 parent 5879d80 commit 965613d
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 5 deletions.
2 changes: 2 additions & 0 deletions src/components/intercept/config/android-device-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
StaticResponseHandler
} from '../../../model/rules/definitions/http-rule-definitions';
import { RulesStore } from '../../../model/rules/rules-store';
import { RulePriority } from '../../../model/rules/rules';

const ConfigContainer = styled.div`
user-select: text;
Expand Down Expand Up @@ -85,6 +86,7 @@ export function setUpAndroidCertificateRule(
id: 'default-android-certificate',
type: 'http',
activated: true,
priority: RulePriority.OVERRIDE,
matchers: [
new MethodMatchers.GET(),
new matchers.SimplePathMatcher(
Expand Down
27 changes: 25 additions & 2 deletions src/components/mock/mock-rule-row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import {
RuleType,
HandlerStep,
isFinalHandler,
isStepPoweredRule
isStepPoweredRule,
RulePriority,
isHttpBasedRule
} from '../../model/rules/rules';
import { ItemPath } from '../../model/rules/rules-structure';
import {
Expand Down Expand Up @@ -187,6 +189,14 @@ const DetailsHeader = styled.div`
margin-bottom: 20px;
`;

const HighPriorityMarker = styled(Icon).attrs(() => ({
icon: ['fas', 'exclamation'],
title: 'High-priority rule: this rule overrides all non-high-prority rules'
}))`
margin-right: 10px;4
align-self: baseline;
}
`;

const RuleMenuContainer = styled(IconMenu)`
background-image: radial-gradient(
Expand Down Expand Up @@ -354,7 +364,13 @@ export class RuleRow extends React.Component<{

// We show the summary by default, but if you set a custom title, we only show it when expanded:
const shouldShowSummary = !collapsed || (!rule.title && !this.titleEditState);

const isEditingTitle = !!this.titleEditState && !collapsed;
const shouldShowCustomTitle = rule.title && !isEditingTitle;

const priorityMarker = isHttpBasedRule(rule) && rule.priority && rule.priority > RulePriority.DEFAULT
? <HighPriorityMarker />
: null;

return <Draggable
draggableId={rule.id}
Expand Down Expand Up @@ -393,8 +409,10 @@ export class RuleRow extends React.Component<{
/>
<DragHandle {...provided.dragHandleProps} />

{ rule.title && !isEditingTitle &&

{ shouldShowCustomTitle &&
<RuleTitle>
{ priorityMarker }
{ rule.title }
</RuleTitle>
}
Expand All @@ -415,6 +433,11 @@ export class RuleRow extends React.Component<{
<MatcherOrHandler>
{ shouldShowSummary &&
<Summary collapsed={collapsed} title={summarizeMatcher(rule)}>
{ !shouldShowCustomTitle &&
// Same condition as the <RuleTitle> block above, because if a
// non-editable title is shown, the marker moves there instead.
priorityMarker
}
{ summarizeMatcher(rule) }
</Summary>
}
Expand Down
2 changes: 2 additions & 0 deletions src/icons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { faExternalLinkAlt } from '@fortawesome/free-solid-svg-icons/faExternalL
import { faPlus } from '@fortawesome/free-solid-svg-icons/faPlus';
import { faMinus } from '@fortawesome/free-solid-svg-icons/faMinus';
import { faExclamationTriangle } from '@fortawesome/free-solid-svg-icons/faExclamationTriangle';
import { faExclamation } from '@fortawesome/free-solid-svg-icons/faExclamation';
import { faLightbulb } from '@fortawesome/free-solid-svg-icons/faLightbulb';
import { faCog } from '@fortawesome/free-solid-svg-icons/faCog';
import { faStar } from '@fortawesome/free-regular-svg-icons/faStar';
Expand Down Expand Up @@ -112,6 +113,7 @@ library.add(
faPlus,
faMinus,
faExclamationTriangle,
faExclamation,
faLightbulb,
faCog,
faStar,
Expand Down
4 changes: 3 additions & 1 deletion src/model/rules/rule-creation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import { getStatusMessage } from '../http/http-docs';
import { RulesStore } from './rules-store';
import {
Handler,
HandlerStep,
HtkMockRule,
RulePriority,
InitialMatcher,
Matcher,
RuleType,
Expand Down Expand Up @@ -212,6 +212,7 @@ export const buildDefaultGroupRules = (
id: 'default-amiusing',
type: 'http',
activated: true,
priority: RulePriority.OVERRIDE,
matchers: [
new HttpRule.MethodMatchers.GET(),
new HttpRule.AmIUsingMatcher()
Expand All @@ -230,6 +231,7 @@ export const buildDefaultGroupRules = (
id: 'default-certificate',
type: 'http' as 'http',
activated: true,
priority: RulePriority.OVERRIDE,
matchers: [
new HttpRule.MethodMatchers.GET(),
new matchers.SimplePathMatcher("amiusing.httptoolkit.tech/certificate")
Expand Down
1 change: 1 addition & 0 deletions src/model/rules/rule-serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ const MockRuleSerializer = serializr.custom(
type: data.type,
title: data.title,
activated: data.activated,
priority: 'priority' in data ? data.priority : undefined,
matchers: data.matchers.map((m) =>
deserializeByType(m, MatcherLookup, context.args)
),
Expand Down
8 changes: 7 additions & 1 deletion src/model/rules/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,4 +462,10 @@ export const isHttpBasedRule = matchRule(isHttpCompatibleType);
export const isWebSocketRule = matchRule(matchRuleType('websocket'));
export const isRTCRule = matchRule(matchRuleType('webrtc'));

export const isStepPoweredRule = isRTCRule;
export const isStepPoweredRule = isRTCRule;

export enum RulePriority {
FALLBACK = 0,
DEFAULT = 1,
OVERRIDE = 2
}
15 changes: 14 additions & 1 deletion test/unit/model/rules-store.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from "../../../src/model/rules/rules-structure";

import { expect } from "../../test-setup";
import { HttpMockRule } from "../../../src/model/rules/definitions/http-rule-definitions";

const group = (id: number, ...items: Array<HtkMockItem>) => ({
id: id.toString(),
Expand All @@ -32,7 +33,7 @@ describe("Rules store", () => {
serverVersion: '1.0.0',
dnsServers: []
};
store = new RulesStore({ featureFlags: [] } as any, proxyStore as any, null as any, null as any);
store = new RulesStore({ featureFlags: [] } as any, proxyStore as any, null as any);
store.rules = { id: 'root', items: [] } as any as HtkMockRuleRoot;
store.draftRules = { id: 'root', items: [] } as any as HtkMockRuleRoot;
});
Expand Down Expand Up @@ -396,5 +397,17 @@ describe("Rules store", () => {
expect(store.rules.items).to.deep.equal([a]);
expect(store.draftRules.items).to.deep.equal([a]);
});

it("should update the rule if it exists but missing its priority", () => {
store.rules.items = [defaultGroup(a)];
store.draftRules.items = [defaultGroup(a)];

const highPriorityRule = { ...a, priority: 10 } as HttpMockRule;

store.ensureRuleExists(highPriorityRule);

expect(store.rules.items).to.deep.equal([defaultGroup(highPriorityRule)]);
expect(store.draftRules.items).to.deep.equal([defaultGroup(highPriorityRule)]);
});
});
});

0 comments on commit 965613d

Please sign in to comment.