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

Bar codes for credential offers on our openid4vci server page. #780

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

sorotokin
Copy link
Contributor

Tested manually

@sorotokin sorotokin requested a review from kdeus November 15, 2024 03:16
class QrServlet : BaseServlet() {
override fun doGet(req: HttpServletRequest, resp: HttpServletResponse) {
val str = req.getParameter("q") ?: ""
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Done


val qr = Encoder.encode(str, ErrorCorrectionLevel.L, null)
val matrix = qr.matrix
val w = matrix.width
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove w and h? We aren't using them everywhere (some places we use matrix.width and matrix.height, other places we use w and h). Let's just be consistent and always use matrix.width, matrix.height.

Or, if we want to consistently use these two variables, rename them to "width" and "height." (generally avoid abbreviations)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, fixed this.

*/
class QrServlet : BaseServlet() {
override fun doGet(req: HttpServletRequest, resp: HttpServletResponse) {
val str = req.getParameter("q") ?: ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to do any sanity checking on the string? Check that it's not 0-length, at least? Or will Encoder.encode already do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zero-length is valid, but missing parameter is worth handling rather than silently ignoring.

@sorotokin sorotokin force-pushed the credential_offer_issues branch from 4518e7b to 3dd0110 Compare November 18, 2024 19:27
@sorotokin sorotokin merged commit 9d2d1dd into main Nov 18, 2024
5 checks passed
@sorotokin sorotokin deleted the credential_offer_issues branch November 18, 2024 20:30
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.

2 participants