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

Optimization for certain API Interfaces #5398

Closed
lxcmyf opened this issue Aug 3, 2023 · 9 comments · Fixed by #5408
Closed

Optimization for certain API Interfaces #5398

lxcmyf opened this issue Aug 3, 2023 · 9 comments · Fixed by #5408
Assignees

Comments

@lxcmyf
Copy link
Contributor

lxcmyf commented Aug 3, 2023

Rationale

Currently, java-tron's HTTP interfaces support GET and POST requests. However, there are some inconsistencies in parameter validation and response between GET and POST calls for the same interface. Here are some examples:

case1: incorrect log level for invalid parameter

/wallet/getapprovedlist
When passing an invalid parameter like modifying type_url to type_url1, the server logs an ERROR:

18:02:57.889 ERROR [qtp464658113-329] [capsule](TransactionCapsule.java:350) Type of the Any message does not match the given class.

case2: inconsistent naming convention for parameters

/wallet/getavailableunfreezecount
image

The GET and POST requests have inconsistent naming for the parameter ownerAddress (GET) vs owner_address (POST).

case3: GET parameter handling contradicts business logic 

/wallet/getcanwithdrawunfreezeamount
image

For the timestamp parameter, the GET request throws NumberFormatException if missing while POST handles it as 0 by default, which aligns with business logic.

/wallet/getcandelegatedmaxsize
image

For the type parameter, GET request throws NumberFormatException if missing while POST handles it as 0 by default, which aligns with business logic.

/wallet/getblockbynum
image
For the num parameter, POST resolves it to 0 without passing it, and GET throws an error. To make it compatible, GET should be consistent with POST.

Implementation

To summarize, 3 cases are listed where GET and POST have inconsistencies. The solutions are as follows:

case1:change log level for invalid parameter validation

case2:standardize naming conventions

case3:align GET handling with POST logic

@lxcmyf lxcmyf changed the title Optimization of Java-tron API Interfaces Optimization for certain API Interfaces Aug 3, 2023
@yuekun0707
Copy link
Contributor

for case 2, should the modification be forward compatible?

@lxcmyf
Copy link
Contributor Author

lxcmyf commented Aug 4, 2023

@yuekun0707
The naming of the get request method parameters for this interface has not previously formed an example document for external users, so users rarely use unofficial incorrect parameters to call the interface. This improvement aims to strive for consistency between the get method and the post method behavior.

@yuekun0707
Copy link
Contributor

yuekun0707 commented Aug 4, 2023

@yuekun0707
The naming of the get request method parameters for this interface has not previously formed an example document for external users, so users rarely use unofficial incorrect parameters to call the interface. This improvement aims to strive for consistency between the get method and the post method behavior.

ok, thanks for your reply~

@eodiandie
Copy link
Contributor

I can see there are both doGet() and doPost() methods in this HTTP API services. from my understanding, there are many common logic that can be used both in doGet() and doPost() methods.
can you separate this common logic and use them in a universe type, which may solve the inconsistencies between GET and POST methods?

@lxcmyf
Copy link
Contributor Author

lxcmyf commented Aug 4, 2023

@xq-lu
You have put forward a very good suggestion. I agree that this is a good idea worth considering.

@asi90sp
Copy link

asi90sp commented Aug 13, 2023

Rationale

Currently, java-tron's HTTP interfaces support GET and POST requests. However, there are some inconsistencies in parameter validation and response between GET and POST calls for the same interface. Here are some examples:

case1: incorrect log level for invalid parameter

/wallet/getapprovedlist When passing an invalid parameter like modifying type_url to type_url1, the server logs an ERROR:

18:02:57.889 ERROR [qtp464658113-329] [capsule](TransactionCapsule.java:350) Type of the Any message does not match the given class.

case2: inconsistent naming convention for parameters

/wallet/getavailableunfreezecount image

The GET and POST requests have inconsistent naming for the parameter ownerAddress (GET) vs owner_address (POST).

case3: GET parameter handling contradicts business logic 

/wallet/getcanwithdrawunfreezeamount image

For the timestamp parameter, the GET request throws NumberFormatException if missing while POST handles it as 0 by default, which aligns with business logic.

/wallet/getcandelegatedmaxsize image

For the type parameter, GET request throws NumberFormatException if missing while POST handles it as 0 by default, which aligns with business logic.

case4:Lack of compatibility handling for parameter

/wallet/getaccount image

The visible parameter for GET only works for base64 encoded addresses, lacking compatibility handling.

case5: POST parameter handling contradicts business logic

/wallet/getblockbynum image

For the num parameter, POST handles it as 0 if missing while GET throws an error, which aligns with business logic.

case6: POST directly invokes GET

/wallet/getnowblock /wallet/listnodes /wallet/getassetissuelist /wallet/listproposals image

For these APIs, POST directly calls GET, so the {visible\:true} input is not handled properly.

case7: GET default handling misaligns with business logic

/wallet/getBrokerage /wallet/getReward image

For address parameter, the default 0 return value contradicts the logic and should throw an error.

Implementation

To summarize, 7 cases are listed where GET and POST have inconsistencies. The solutions are as follows:

case1:change log level for invalid parameter validation

case2:standardize naming conventions

case3:align GET handling with POST logic

case4:add compatibility handling for GET parameters

case5:validate POST parameters to match GET logic

case6:modify POST response based on input

case7:throw error for invalid GET parameters

Ok

@halibobo1205
Copy link
Contributor

Keep in mind to keep compatibility.

@halibobo1205
Copy link
Contributor

halibobo1205 commented Aug 16, 2023

Will application/x-www-form-urlencoded be supported?

@lxcmyf
Copy link
Contributor Author

lxcmyf commented Aug 18, 2023

@halibobo1205

Keep in mind to keep compatibility.

Of course.

Will application/x-www-form-urlencoded be supported?

Yes, it will be compatible with this content-type.

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

Successfully merging a pull request may close this issue.

5 participants