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

get_method, get_uri on request::Builder #282

Closed
kazuk opened this issue Dec 6, 2018 · 6 comments
Closed

get_method, get_uri on request::Builder #282

kazuk opened this issue Dec 6, 2018 · 6 comments

Comments

@kazuk
Copy link
Contributor

kazuk commented Dec 6, 2018

I am writing Azure Storage REST Client and implementing Authorize with Shared Key. To create signatures, I need HTTP Method(VERB) , header values, and request URI on the current request. (See Blob, Queue, and File Services (Shared Key authorization))

I wish to add functions to request::Builder

  • get_method
  • get_uri
  • get_headers

then I can implement sign_request as bellow

fn sign_request(&self, request_builder: Builder ) -> Builder {
    // build string_to_sign
    let string_to_sign =format!( "{}\n.....", request_builder.get_method(). ... );
    // compute signature 
    let signature = ....;
    // add Authorization header to Builder and return it.
    request_builder.header( AUTHORIZATION, "SharedKey {}:{}", self.account_name,signature )
}

Can you accept if I send changes?

Or is there any security reason that request::Builder shouldn't provide HTTP method etc?

@seanmonstar
Copy link
Member

On one hand, it certainly seems reasonable to want to view what fields have already been set. On the other, I can't remember any other builder patterns where getters are included...

@kazuk
Copy link
Contributor Author

kazuk commented Dec 7, 2018

https://en.wikipedia.org/wiki/Builder_pattern

C# builder having getter. because C# property MUST have getter.

C++,Java,etc builder not having getter.
But user can view current value from builder and update as bellow.

let temp_order = builder.build();
let tax = compute_tax( temp_order.item_lines() );
let taxed_order = builder.set_tax(tax).build();

in Rust, Builder consumed on first line build() and ends life. last line build() to make compilation error.

  • from 'C# Builder' example, Builder have getter when language requirement or limitation hits situation.
  • in Rust, Builder causes some of compilation errors, user do 'view current value'.

IMHO
'some of compilation errors' as 'language requirement or limitation hits situation' , and then Builder can have getter.

@carllerche
Copy link
Collaborator

I've wanted this every so often. I would be fine with having get_* functions.

@carllerche
Copy link
Collaborator

Though, if we are to bikeshed a bit, generally Rust doesn't have get_* prefix. We obviously can't have no prefix since that fn is used by the setter...

I'm leaning towards just adding get_ prefixes, but I'm open to other thoughts.

@kazuk
Copy link
Contributor Author

kazuk commented Dec 13, 2018

Thank you ,
I send pull request #284,

generally Rust doesn't have get_* prefix. We obviously can't have no prefix since that fn is used by the setter...

Yes, and Yes,
get_ prefix for avoid the breaking changes.

@kazuk
Copy link
Contributor Author

kazuk commented Dec 19, 2018

Merged! 🎉

@kazuk kazuk closed this as completed Dec 19, 2018
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

No branches or pull requests

3 participants