mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-07 02:37:14 +00:00
Revert "fix(routing): insert new rules after api instead of appending at the end"
This reverts commit 638458bd. Reordering new rules to the top by
creation-time was a blunt heuristic: it fixed a rule silently shadowed
by an unrestricted catch-all above it, but breaks the opposite,
equally valid setup -- e.g. a pre-existing "youtube -> interfaceA" rule
(no inboundTag restriction, meant to apply everywhere) followed by a
new "amneziawg -> interfaceB" rule. Under the reverted logic the newer,
broader-matching amneziawg rule would jump above the youtube rule and
shadow it for amneziawg-sourced traffic -- the exact same class of bug
this was meant to fix, just aimed the other way.
Xray's rule order encodes real admin intent (which rule should win for
overlapping traffic) that creation time can't safely stand in for.
Back to plain append-at-the-end; admins reorder manually with the
existing moveUp/moveDown controls, same as upstream's own behavior.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -21,7 +21,7 @@ import RuleFormModal from './RuleFormModal';
|
||||
import type { RoutingRule } from './RuleFormModal';
|
||||
import RuleCardList from './RuleCardList';
|
||||
import { useRoutingColumns } from './useRoutingColumns';
|
||||
import { arrJoin, originalRuleIndex, isApiRule } from './helpers';
|
||||
import { arrJoin, originalRuleIndex } from './helpers';
|
||||
import type { RuleRow } from './types';
|
||||
import type { XraySettingsValue, SetTemplate } from '@/hooks/useXraySetting';
|
||||
import type { RuleObject } from '@/schemas/routing';
|
||||
@@ -209,20 +209,8 @@ export default function RoutingTab({
|
||||
if (!tt.routing) tt.routing = { rules: [] };
|
||||
if (!Array.isArray(tt.routing.rules)) tt.routing.rules = [];
|
||||
const typed = rule as unknown as RuleObject;
|
||||
if (editingIndex == null) {
|
||||
// Rules match top-to-bottom, first hit wins, so a brand-new rule
|
||||
// appended at the end is silently shadowed by any earlier
|
||||
// broader/catch-all rule that also happens to match its traffic —
|
||||
// a real trap: the rule looks saved and enabled, but never actually
|
||||
// fires. Insert it as early as possible instead (right after the
|
||||
// pinned api rule, if present) so a newly created rule takes effect
|
||||
// by default; the admin can still drag it lower with moveDown if
|
||||
// that's genuinely what they want.
|
||||
const insertAt = isApiRule(tt.routing.rules[0] as RuleObject) ? 1 : 0;
|
||||
tt.routing.rules.splice(insertAt, 0, typed);
|
||||
} else {
|
||||
tt.routing.rules[editingIndex] = typed;
|
||||
}
|
||||
if (editingIndex == null) tt.routing.rules.push(typed);
|
||||
else tt.routing.rules[editingIndex] = typed;
|
||||
});
|
||||
setRuleModalOpen(false);
|
||||
}
|
||||
|
||||
@@ -1,62 +0,0 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { fireEvent, screen } from '@testing-library/react';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
|
||||
import RoutingTab from '@/pages/xray/routing/RoutingTab';
|
||||
import type { XraySettingsValue } from '@/hooks/useXraySetting';
|
||||
|
||||
import { renderWithProviders } from './test-utils';
|
||||
|
||||
function settingsWithApiAndBlockRules(): XraySettingsValue {
|
||||
return {
|
||||
routing: {
|
||||
rules: [
|
||||
{ type: 'field', inboundTag: ['api'], outboundTag: 'api', enabled: true },
|
||||
{ type: 'field', ip: ['ext:geoip_RU.dat:ru'], outboundTag: 'blocked', enabled: true },
|
||||
{ type: 'field', protocol: ['bittorrent'], outboundTag: 'blocked', enabled: true },
|
||||
],
|
||||
},
|
||||
} as unknown as XraySettingsValue;
|
||||
}
|
||||
|
||||
// Rules match top-to-bottom, first hit wins. A brand-new rule used to be
|
||||
// appended at the end, where a pre-existing broader/catch-all rule (e.g. a
|
||||
// block rule with no inboundTag restriction, like the ones here) silently
|
||||
// shadows it forever -- the rule looks saved and enabled but never actually
|
||||
// fires. See RoutingTab.tsx onRuleConfirm.
|
||||
describe('RoutingTab new-rule insert position', () => {
|
||||
it('inserts a newly created rule right after the pinned api rule, not at the end', () => {
|
||||
const setTemplateSettings = vi.fn();
|
||||
const initial = settingsWithApiAndBlockRules();
|
||||
|
||||
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
renderWithProviders(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<RoutingTab
|
||||
templateSettings={initial}
|
||||
setTemplateSettings={setTemplateSettings}
|
||||
inboundTags={['awg-tag']}
|
||||
clientReverseTags={[]}
|
||||
isMobile={false}
|
||||
/>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('tab', { name: /Routing Rules/ }));
|
||||
fireEvent.click(screen.getByRole('button', { name: /Routing Rules/ }));
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Create' }));
|
||||
|
||||
expect(setTemplateSettings).toHaveBeenCalledTimes(1);
|
||||
const updater = setTemplateSettings.mock.calls[0][0] as (prev: XraySettingsValue) => XraySettingsValue;
|
||||
const next = updater(initial);
|
||||
const rules = (next.routing as { rules: Array<{ inboundTag?: string[]; outboundTag?: string }> }).rules;
|
||||
|
||||
expect(rules.length).toBe(4);
|
||||
expect(rules[0].outboundTag).toBe('api');
|
||||
// The two pre-existing block rules must have been pushed down, not the
|
||||
// new rule appended after them.
|
||||
expect(rules[1].outboundTag).not.toBe('blocked');
|
||||
expect(rules[2].outboundTag).toBe('blocked');
|
||||
expect(rules[3].outboundTag).toBe('blocked');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user