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

Grpc-Metadata- functionality not matching documentation #245

Closed
hoffin opened this issue Oct 22, 2016 · 6 comments
Closed

Grpc-Metadata- functionality not matching documentation #245

hoffin opened this issue Oct 22, 2016 · 6 comments
Labels

Comments

@hoffin
Copy link

hoffin commented Oct 22, 2016

Mapping of HTTP request headers doesn't work as described in the documentation.

Remaining HTTP header keys are prefixed with Grpc-Metadata- and added with their values to gRPC request header

The Documentation asserts that all remaining headers will have Grpc-Metadata- prepended and be available to the gprc endpoint but in fact it only passes through headers that arrive with Grpc-Metadata- prepended already.

I'm looking to add the REST service behind kong API which has fixed headers added to the upstream call which I would like to access at the grpc endpoint. Options.

Options are..

  • Make change to code to make it work as described.
  • Change documentation to reflect actual working (such as)

Remaining HTTP header keys that are prefixed with Grpc-Metadata- will be added with their values to gRPC request header

Happy to do either but as I'm new to this project I've not managed to go through all the prior thinking.

@philipithomas
Copy link
Contributor

philipithomas commented Oct 22, 2016

You're correct. I added the incorrect documentation as I was getting up and running with the project, but this week I realized that it was incorrect. I think the fix is updating documentation, and your change looks good. I would just perhaps add that "all HTTP headers not prefixed with Grpc-Metadataare dropped."

However - I think there could be some possible security implications with this behavior. It means that an attacker can selectively inject gRPC metadata with a correctly-crafted request. Depending on how metadata is used, I think this could be scary.

@hoffin
Copy link
Author

hoffin commented Oct 22, 2016

Right-o. Is there a know tricks in getting a non Grpc-Metadata- request headers into the gRPC endpoint within the reverse proxy code without manipulating the headers before it arrives, as these headers are not always available to be set at source.

Just to ask, is there a specific reason why it wasn't implemented in the way it was described in the documentation as that would solve my issue without having to manipulate the headers before they arrive at the reverse proxy?

@philipithomas
Copy link
Contributor

The documentation was written after implementation. However, I think that the documented behavior is more secure than the actual one :-) @yugui - what are your thoughts?

@hoffin
Copy link
Author

hoffin commented Oct 22, 2016

I would much prefer the documented method. There's a lot less fiddling to get access to the header data and much simpler integration with api gateways.

@hoffin hoffin changed the title Grpc-Metadata- functionality not matching documentaiton Grpc-Metadata- functionality not matching documentation Oct 23, 2016
@tmc
Copy link
Collaborator

tmc commented Nov 4, 2016

see #213 for some related discussion

ithinker1991 pushed a commit to tronprotocol/grpc-gateway that referenced this issue Apr 26, 2018
ithinker1991 added a commit to tronprotocol/grpc-gateway that referenced this issue May 11, 2018
e92f847 Merge pull request grpc-ecosystem#46 from tronprotocol/rate_limit
e191aed add grpc return type:server busy
c98bb68 Merge pull request grpc-ecosystem#45 from tronprotocol/add_expiration
5abd2c0 fix spell error
76d2ac1 Merge pull request grpc-ecosystem#44 from tronprotocol/add-reason-enum
cf23e93 Merge pull request grpc-ecosystem#42 from tronprotocol/add_witness_committee
e9b0876 add the reason code
cc24179 Merge pull request grpc-ecosystem#43 from tronprotocol/add_expiration
5f9e263 add transaction expiration erro
3913331 fix merge conflict
bb1f7e0 add message Vote
4e0f34e Merge pull request grpc-ecosystem#40 from tronprotocol/proxy
7134384 minor change
ab77170 add witness and committee in account
8f7655a add error code for broadcast.
4bc508c fix build error.
a568eb8 add response code for return of broad_cast && fix error.
4dc6857 add response code for return of broad_cast.
df2ea4b Merge pull request grpc-ecosystem#41 from tronprotocol/add-reason-enum
54dac0c fix-reason
200ba66 proxy support GET method
0e65a14 Merge pull request grpc-ecosystem#39 from tronprotocol/evan-rm-createaccount
d57ad8f remove createaccount api
e13211c Merge pull request grpc-ecosystem#38 from tronprotocol/evan-rm-transaction-type
8aeba00 rm transaction type
c7495f8 Merge pull request grpc-ecosystem#36 from tronprotocol/add-reason-enum
59333a3 add reason
9aa4212 Merge pull request grpc-ecosystem#35 from tronprotocol/add_fullNode_API
e4025ff feat: delete GetTransactionByLimitPrev.
07d94e4 Merge pull request grpc-ecosystem#34 from tronprotocol/add_fullNode_API
884b666 fix: add http option.
5d93c0c feat: add fullnode GRPC.
08b0663 Merge commit 'c3b07944ee8eb47d7461fd304520364db41671ad' into develop
c3b0794 Merge pull request grpc-ecosystem#33 from tronprotocol/add_frozen_allowance
b7224ed add bandwidth in account
3464107 Merge branch 'develop' of https://github.com/tronprotocol/java-tron into develop
9313380 Merge pull request grpc-ecosystem#32 from tronprotocol/add_frozen_allowance
88da776 minor change
f12e2cc minor change
d18ae9c minor change
e18bdf2 add frozen and allowance balance
479564d mdf channel
f5e7632 Merge commit '647650de48d27934e0eb018db1c0ff4242ee39a5' into develop
647650d Merge pull request grpc-ecosystem#31 from tronprotocol/tx-timestamp
b290aad add hello message timestamp
6eba349 fix the transfer exception.
01a4166 Merge pull request grpc-ecosystem#30 from tronprotocol/update_account
80c5fd4 Merge branch 'master' into update_account
6cb5e8c Merge pull request grpc-ecosystem#528 from tronprotocol/refactor_transaction
7fc51a7 refactor the transaction structure
221cf70 Merge pull request grpc-ecosystem#525 from tronprotocol/refactor_transaction
7308a22 refactor the transaction structure
3868626 merge develop
a68e403 Merge commit 'd49019ec76c606ab1aa743a97710bac8b291cbf5' into add_grpc_api_for_solidity
d49019e Merge pull request grpc-ecosystem#28 from tronprotocol/fix_return
ff425d8 fix: return type.
1435bef Merge pull request grpc-ecosystem#27 from tronprotocol/add-reason-enum
2ec383f add ReasonCode
4d6bbda modify use LatestBlockHeaderNumber compare
d139597 modify use LatestBlockHeaderNumber compare
bc9ca33 merge protocol
8c6167e Merge pull request #25 from tronprotocol/xd-reason-code
130f5a2 modify reason code
6f41dfa Merge pull request #22 from tronprotocol/feature/add_db_api
9498448 feature: merge master to this.
13d4b62 Merge remote-tracking branch 'origin/master' into feature/add_db_api
246cf61 Merge pull request #24 from tronprotocol/add_database_api
2f7ae0b feature:add Datebase api Service.
a5e544c fix:change go_package.
c003d39 Merge remote-tracking branch 'origin/master' into feature/add_db_api
2dbee3b add WalletSolidity api service.
acc5c20 add get AssetIssueList by Timestamp api.
87c23fb add get Transaction api.
1033d54 Merge branch 'develop' into remove_header
c56402e add solidity node impl
f549760 add go package setting
7f1d31a add go package setting
0ef1de2 Merge branch 'develop' of https://github.com/tronprotocol/java-tron into develop
303dd5d feat: add BlockReference api
ed93332 Merge commit '1fab36b414599a3b24e739da4f04ee57bb4c14a5' into develop
d4a93ed improve: modified the seq num and fix build error
7acdfaf improve: add ref_block_num and ref_block_hash
c7f7d3e feat: remove utxo support from protocol.
59a3f02 Merge remote-tracking branch 'origin/develop' into develop
ac5eed9 add comments about ParticipateAssetIssueContract.
b1f5964 Merge commit 'ce4933fbe83c734ca244ecb0e8f8a9f854aa8afe' into protocol
336711d Merge commit '0297ff5d8a027576b0a7943cf0174ca7fc789534' into develop
f7d6d7b Merge commit 'ba4df0279d39dc07b6f6e40ccb0a66f3f5631e67' into feature/add_total_trans
0ed107a Merge commit '9f390c30504fe635e9d968393c2869f14cdfe9a4' into develop
4842483 Merge commit '06d60bdde8214ae47fecd8625e31dcd54c0bb324' into p2p
95b951b Merge branch 'develop' into p2p
192b1df Revert "Revert "Feature/devfor evan""
4e5ce06 Merge commit '9ac2d9b122b1e5ce429b4cc607741c56fbe27d4c' into p2p
fcc01fe Revert "Feature/devfor evan"
222e9e0 Merge remote-tracking branch 'origin/feature/devforEvan' into feature/devforEvan
0e407b9 merage proto
abb7a8b Merge commit '4668a8e92da61efdcb12d1bc7275d02373a7e052' into feature/devforEvan
e0dd5af Merge commit 'ca59ee900e2611553abebbf3a9f8feacab52228e' into feature/devforEvan
3f78d1b merge
94005a1 Merge commit 'ca59ee900e2611553abebbf3a9f8feacab52228e' into p2p
2836bdd change getBalance to getAccount
a97cfd7 Merge commit 'f1a1e24144dcefa5b604aa4a5ed69ef3bb934c66' into p2p
473587b merage proto
b74432d Merge remote-tracking branch 'origin/feature/devforEvan' into feature/devforEvan
312121f merage proto
faabdc4 Merge commit '044a39a4666b0428785d0501067bf417ade820c1' into p2p
82ceb8d Merge commit '13f1658d0497ecf4b6ce3ae9ed199b35cf654e21' into feature/devforEvan
f6ecbb5 Merge commit '0d9e0fdb313a8d4d5d3baaaea22569b72645fc08' into feature/devforEvan
8d0790f Merge commit 'bbeed13b2b052ae0fc669fb4a4ce1d5ac2cc9092' into p2p
8140964 Merge commit 'd08db0e07224a7aac3f296fc3a2e930e24d06fb5' into p2p
0aaff3d mdf discover msg
cf487bb Merge commit 'f63eda3db1b4386128cbc6235c3b95a72fbf0ba0' into p2p
dc9c1bb Merge commit '79c9b333f22367d3792f3faa2970b8dbbcf646b0' into feature/devforEvan
7abfb54 Merge commit '0f3ab59dfcba7e39789cdb810f3bd69a4fabb44b' into feature/devforEvan
5b0f8ac add getBlock
8901f2c add discover message
936b701 Merge commit '2f82ce89fcbebced73126885cf2feeda3463c73d' into develop
bbd0ea3 Merge commit '050740d1801ad990f2ad3c9eeee3030c74e022f3' into p2p
61decfb Merge commit '2795379f4fee8b12715e8835967f67925a5d68e7' into p2p
b5ced2d Merge branch 'develop' into junit/config
68b5a48 Merge pull request grpc-ecosystem#251 from sdargutev/develop
6b70dfd Merge pull request grpc-ecosystem#245 from tronprotocol/feature/updatewitness
935fd4c merged
c9410d8 refactoring
a3c7257 Merge remote-tracking branch 'origin/develop' into junit/config
9df3d33 Merge commit 'ec85aa1c2f62dbb6a027cd713e5965311dee29e6' into feature/updatewitness
fee98bf Merge remote-tracking branch 'origin/syncBlockChain' into syncBlockChain
7f4f302 rm duplicate instance
f509ce3 Merge branch 'develop' of https://github.com/tronprotocol/java-tron into syncBlockChain
774cceb Merge pull request grpc-ecosystem#237 from tronprotocol/miss_confuse_db
164d8bd remove dupicated number refresh
a58bf8e Merge branch 'develop' into junit/config
bb3a578 improve code readability
6910ef4 fix: add Account setter() validator

git-subtree-dir: protocol
git-subtree-split: e92f847
@stale
Copy link

stale bot commented Sep 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 9, 2019
@stale stale bot closed this as completed Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants