-
Notifications
You must be signed in to change notification settings - Fork 13
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
Issue #6 - support for custom headers #7
Conversation
…n the global options setup
Hi @bartek4c, this was meant to be available via CustomWkHtmlPageArgs (string) on PdfConversionSettings (however, that is indeed broken for this usage). But I agree it could be better than a string. I really need to just go ahead and wrap up Page, Cover, Header, Footer, Outline, and TOC in their own configuration objects, but this was supposed to be a "simple" wrapper, heh. You're placing this collection of config options as if they are Global options, not a Page, Header, Footer, etc Option. And while that works for custom header, it might not work or have the desired effect for all usage scenarios. For example, having a custom header that is different for two pages instead of forcing that header for all pages (how you have it). I just did a release for the day job, so please give me a couple days and I'll work up something more complete to map all the options. |
Hi @cp79shark,. I think we are talking about two different things here. You mean the html args while I mean the HTTP headers which have nothing to do with html. There is a good collection of custom headers which you might wish to pass with your request, hence the dictionary object (examples here https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers). In my case I had to pass the Authorization header to pass a bearer token. Here is another example of use https://stackoverflow.com/questions/20522933/add-a-header-to-wkhtmltoimage. Hope it makes sense |
Nope, we are talking about the same thing, my response is just wider in scope than custom HTTP headers. I'm talking about all options, not just --custom-header. Don't be confused by the property name Some pseudo code for what I'm talking about. Obviously there would be sensible defaults. Not entirely sure how I want to do it yet, but it should 1. Expose all options so you don't need to use magic strings and 2. Be Fluent / Builder Pattern like for ease of use. What do you think? Again, I won't be able to get to this until the end of the week. The day job has a monopoly on my time at the moment.
|
Ah ok. That probably makes more sense and makes the solution more flexible. Thanks for the feedback. My changes seem to be working for me for now, so I'm looking forward to see your implementation whenever it comes. |
Hi @cp79shark. I have done two small additions to your code to support my use case described in issue #6. First I have added a dictionary for custom headers to the settings class. Later on, in PdfConvert class, I iterate through the dictionary object to create additional options for WkHtmlToPdf tool. I have used this solution for the authorization header to pass a bearer token together with my request and it works perfectly fine. I have tested it with other headers types as well, and also tested adding multiple headers at the same time.
This is an example of using it:
settings.CustomHeaders.Add("Authorization", $"Bearer {accessToken}");