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

Api for lack middleware #205

Merged
merged 8 commits into from
Aug 9, 2022

Conversation

shakatoday
Copy link
Contributor

Brief

  1. CLOG:INITIALIZE - added keyword parameter :lack-middleware-list to prepend Lack Middlewares.
  2. Coding style tweak.
  3. Fixed an emergent bug after adding the Lack Middleware api.

More Info

API for Prepending Lack Middleware

I want it. Please let me add this if you think that's ok. 😁

Coding style tweak

  1. use (or form-1 form-2) instead of (if form-1 form-1 form-2)
  2. auto indented
  3. Fewer duplicate symbols.
    Above 3 commits are coding style tweaks. Feeling free to tell me if there's something you don't like. Or, you can directly modify it, but remember to add the new argument pair (if you agree) :lack-middleware-list lack-middleware-list when calling CLOG-CONNECTION:INITIALIZE as your original style.

Fixed an emergent bug after adding the Lack Middleware api.

Environments

Operating System & Hardware: MacOS Monterey 12.5 with M1 Pro
Common Lisp Implementation: SBCL 2.2.5
Browser: Safari Version 15.6 (17613.3.9.1.5)

Reproduction

The bug had appeared whenever adding more than 1 Lack Middlewares by :lack-middleware-list.
If anyone is interested, you here's the branch to reproduce the bug.
error-test-case-when-x-www-form-urlencoded-and-more-middlewares

;; Run my modified version of tutorial 17
CL-USER> (clog:run-tutorial 17)
To load "lack-middleware-accesslog":
  Load 1 ASDF system:
    lack-middleware-accesslog
; Loading "lack-middleware-accesslog"

To load "lack-middleware-session":
  Load 1 ASDF system:
    lack-middleware-session
; Loading "lack-middleware-session"

Then submit anything via the Post Form
Screen Shot 2022-08-09 at 2 52 03 PM

Then the server side error

The value
  121
is not of type
  CHARACTER
when setting an element of (ARRAY CHARACTER)
   [Condition of type TYPE-ERROR]

Fix

This commit fixed the error. fixed read-sequence type error when there are more lack middlewares

I didn't add circular-streams into clog.asd because lack-request has already depended on circular-streams,

On this branch error-fixed-with-test-case-for-api-for-lack-middleware you can test the patch since it also contains a modified version of tutorial 17 with 2 prepended lack middlewares.

It could be better to decompose clog-connection:initialize into multiple
functions which clog-system:initialize can call.
Error message: The value [NUMBER] is not of type CHARACTER when setting an element of (ARRAY CHARACTER).
Thus, the updated code checks the type of RAW-BODY to decide what to do.
@rabbibotton
Copy link
Owner

All looks good to me merging.

Since CLAK is the only "lgpl" portion of clog and that creates some issues for some companies, we will likely have to support a hunchentoot direct version in the future. I have had two requests already. One already sent two emails with no response so far to Fukamachi to see if would consider switching license. Don't know if you have a better way to reach, would be good to leave as is.

@rabbibotton rabbibotton merged commit 47b869f into rabbibotton:main Aug 9, 2022
@shakatoday shakatoday deleted the api-for-lack-middleware branch August 10, 2022 00:40
@shakatoday
Copy link
Contributor Author

shakatoday commented Aug 10, 2022

Proprietary softwares can include LGPL libraries. There should be no problems. Could you explain the issues a bit?
https://twitter.com/nitro_idiot
As far as I know, Fukamachi is probably active on Twitter.
I only met him in two Common Lisp activities in Japan, so I guess I can't do the referral well.

I'll send this URL about our communication to another Lisper I'm more familiar with, who is much closer to Fukamachi.

@rabbibotton
Copy link
Owner

rabbibotton commented Aug 10, 2022 via email

@shakatoday
Copy link
Contributor Author

shakatoday commented Aug 10, 2022

Thank you for explanation.
The licenses of those libraries are LLGPL provided by U.S. based Franz Inc.
As U.S. being a Common Law country, I guess the problem could be that for Lisp libraries with LLGPL (or LGPL) there have been not enough precedents on the court.

This legal issue is too big to me. I've sent the URL of this thread to the Lisper I mentioned, who is close to Fukamachi.

I think Fukamachi will reply tweets & messages on his Twitter, though. https://twitter.com/nitro_idiot

@fukamachi
Copy link

Hi, I'm the author of Clack/Lack.

I don't think the LLGPL license will be any legal problems while Clack and Lack are used just as a library, as Shaka said.
However, I learned that some people tend to dislike GPL-like licenses emotionally, not legally.

So, I changed those licenses to MIT.
We are in a small community, so let's help where we can.

@rabbibotton
Copy link
Owner

I agree :)

Truly appreciated! Thanks!

@rabbibotton
Copy link
Owner

rabbibotton commented Oct 11, 2022 via email

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.

3 participants