Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support external controller and its override settings #299

Merged
merged 1 commit into from
Aug 31, 2024

Conversation

karin0
Copy link

@karin0 karin0 commented Aug 24, 2024

This adds initial support for external controller (RESTful APIs). The inbound for external controller is started when loading a profile, and override settings for external_controller and secret are added.

The default value for external_controller is set to 127.0.0.1:0, indicating disabled for now, to keep compatibile behaviors and to avoid taking settings from untrusted profiles.

Still, due to how the external controller is implemented, I haven't found a way to update the inbound when external_controller and secret are changed in the profile or settings, so a restart (force stop of the app) is required to apply the changes for now.

@xishang0128 xishang0128 force-pushed the external-controller branch 2 times, most recently from 5fd0e0b to fc9bb97 Compare August 31, 2024 09:24
@karin0 karin0 force-pushed the external-controller branch from fc9bb97 to bacc753 Compare August 31, 2024 09:48
@karin0
Copy link
Author

karin0 commented Aug 31, 2024

I have rebased the changes onto the current upstream.

@xishang0128 xishang0128 merged commit d62430a into MetaCubeX:main Aug 31, 2024
1 check passed
@karin0 karin0 deleted the external-controller branch August 31, 2024 10:03
@wwqgtxx
Copy link

wwqgtxx commented Sep 10, 2024

Can you confirm that the overriding of ExternalController works properly?

In my actual test, no overriding occurred and the settings in the configuration file were directly loaded.

@karin0
Copy link
Author

karin0 commented Sep 10, 2024

Can you confirm that the overriding of ExternalController works properly?

In my actual test, no overriding occurred and the settings in the configuration file were directly loaded.

Yes, I'm using v2.10.4 release, and my profile contains valid external-controller and secret fields. After setting different values in Settings > Override and restarting Clash, I can successfully access the external controller using my specified port and secret in the Override.

@wwqgtxx
Copy link

wwqgtxx commented Sep 10, 2024

Actually if you keep the default value of 127.0.0.1:0 unchanged, it will not override the setting in the config file.
REF: https://stackoverflow.com/questions/69008915/how-to-serialize-kotlin-data-class-with-default-values-into-json-using-kotlinx-s

@karin0
Copy link
Author

karin0 commented Sep 10, 2024

Actually if you keep the default value of 127.0.0.1:0 unchanged, it will not override the setting in the config file. REF: https://stackoverflow.com/questions/69008915/how-to-serialize-kotlin-data-class-with-default-values-into-json-using-kotlinx-s

That's indeed the case. Shall I add the annotation @EncodeDefault to fix this?

@wwqgtxx
Copy link

wwqgtxx commented Sep 10, 2024

Actually if you keep the default value of 127.0.0.1:0 unchanged, it will not override the setting in the config file. REF: https://stackoverflow.com/questions/69008915/how-to-serialize-kotlin-data-class-with-default-values-into-json-using-kotlinx-s

That's indeed the case. Shall I add the annotation @EncodeDefault to fix this?

This should be a solution, I just wonder if adding this annotation will have other unknown side effects.

@karin0
Copy link
Author

karin0 commented Sep 10, 2024

Actually if you keep the default value of 127.0.0.1:0 unchanged, it will not override the setting in the config file. REF: https://stackoverflow.com/questions/69008915/how-to-serialize-kotlin-data-class-with-default-values-into-json-using-kotlinx-s

That's indeed the case. Shall I add the annotation @EncodeDefault to fix this?

This should be a solution, I just wonder if adding this annotation will have other unknown side effects.

Then to avoid taking the fields like externel-controller from the profile, could we instead discard them entirely by setting their original values to be empty during UnmarshalAndPatch (or related flows like patchOverride), before the override values are applied? This way, only the explicit override values would be kept, so the default override values can remain null with no need for @EncodeDefault.

@wwqgtxx
Copy link

wwqgtxx commented Sep 10, 2024

Actually if you keep the default value of 127.0.0.1:0 unchanged, it will not override the setting in the config file. REF: https://stackoverflow.com/questions/69008915/how-to-serialize-kotlin-data-class-with-default-values-into-json-using-kotlinx-s

That's indeed the case. Shall I add the annotation @EncodeDefault to fix this?

This should be a solution, I just wonder if adding this annotation will have other unknown side effects.

Then to avoid taking the fields like externel-controller from the profile, could we instead discard them entirely by setting their original values to be empty during UnmarshalAndPatch (or related flows like patchOverride), before the override values are applied? This way, only the explicit override values would be kept, so the default override values can remain null with no need for @EncodeDefault.

I think this solution is feasible. We just need to add a function before patchOverride to clear the value.

@karin0
Copy link
Author

karin0 commented Sep 11, 2024

Actually if you keep the default value of 127.0.0.1:0 unchanged, it will not override the setting in the config file. REF: https://stackoverflow.com/questions/69008915/how-to-serialize-kotlin-data-class-with-default-values-into-json-using-kotlinx-s

That's indeed the case. Shall I add the annotation @EncodeDefault to fix this?

This should be a solution, I just wonder if adding this annotation will have other unknown side effects.

Then to avoid taking the fields like externel-controller from the profile, could we instead discard them entirely by setting their original values to be empty during UnmarshalAndPatch (or related flows like patchOverride), before the override values are applied? This way, only the explicit override values would be kept, so the default override values can remain null with no need for @EncodeDefault.

I think this solution is feasible. We just need to add a function before patchOverride to clear the value.

OK, I have opened #330 and it's tested to resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants