Copilot commented on code in PR #4163:
URL: https://github.com/apache/hertzbeat/pull/4163#discussion_r3520215498
##########
web-app/src/app/routes/monitor/monitor-form/monitor-form.component.ts:
##########
@@ -265,6 +267,19 @@ export class MonitorFormComponent implements OnChanges {
}
}
});
+ if (dependField === 'httpMethod') {
+ this.clearPayloadIfHttpMethodHasNoBody(dependValue);
+ }
+ }
+
+ private clearPayloadIfHttpMethodHasNoBody(httpMethod: unknown): void {
+ if (httpMethod == null || ['POST', 'PUT',
'PATCH'].includes(String(httpMethod).toUpperCase())) {
+ return;
+ }
Review Comment:
The monitor definitions and collector implementation only treat request
bodies as applicable for POST/PUT (e.g., `define/app-api.yml` and
`HttpCollectImpl` only attach payload for POST/PUT). Including PATCH here means
a stale payload can be preserved for a method where the UI/backend don’t expect
to send a body, which can reintroduce the “hidden body” problem for PATCH-like
methods.
Consider aligning this allow-list to POST/PUT only (or update the
backend/definitions if PATCH bodies are intended to be supported).
##########
web-app/src/app/routes/monitor/monitor-form/monitor-form.component.spec.ts:
##########
@@ -17,27 +17,56 @@
* under the License.
*/
-import { ComponentFixture, TestBed } from '@angular/core/testing';
-
import { MonitorFormComponent } from './monitor-form.component';
describe('MonitorFormComponent', () => {
let component: MonitorFormComponent;
- let fixture: ComponentFixture<MonitorFormComponent>;
-
- beforeEach(async () => {
- await TestBed.configureTestingModule({
- declarations: [MonitorFormComponent]
- }).compileComponents();
- });
beforeEach(() => {
- fixture = TestBed.createComponent(MonitorFormComponent);
- component = fixture.componentInstance;
- fixture.detectChanges();
+ component = new MonitorFormComponent(
+ {
+ info: () => undefined,
+ error: () => undefined
+ } as any,
+ {
+ fanyi: (key: string) => key
+ } as any
+ );
+ component.monitor = { name: 'api monitor', scheduleType: 'interval' } as
any;
+ component.collector = '';
+ component.sdParams = [];
+ component.paramDefines = [];
+ component.sdDefines = [];
+ component.advancedParamDefines = [{ depend: { httpMethod: ['POST'] } } as
any];
});
it('should create', () => {
expect(component).toBeTruthy();
});
+
+ it('should clear payload when httpMethod changes to a method without body',
() => {
+ component.advancedParams = [{ field: 'payload', paramValue: 'old body' }
as any];
+
+ component.onDependChanged('GET', 'httpMethod');
+
+ expect(component.advancedParams[0].paramValue).toBe('');
+ });
+
+ it('should keep payload when httpMethod changes to POST-like methods', () =>
{
+ component.advancedParams = [{ field: 'payload', paramValue: 'old body' }
as any];
+
+ component.onDependChanged('POST', 'httpMethod');
+
+ expect(component.advancedParams[0].paramValue).toBe('old body');
+ });
Review Comment:
This test only verifies that POST preserves the payload. The monitor
definition for `payload` is dependent on `httpMethod: [POST, PUT]`, so it’s
worth asserting PUT as well to prevent regressions when adjusting the
allowed-method list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
