amishmm / php-pam

This extension provides PAM (Pluggable Authentication Modules) integration for PHP
Other
10 stars 5 forks source link

add type hinting with PHP 8 #5

Closed remicollet closed 3 years ago

remicollet commented 3 years ago

Take benefit of PHP 8 feature: generate arginfo from stub.php

This also add type hints

Previous version

    Function [ <internal:pam> function pam_auth ] {

      - Parameters [5] {
        Parameter #0 [ <required> $username ]
        Parameter #1 [ <required> $password ]
        Parameter #2 [ <optional> &$status ]
        Parameter #3 [ <optional> $checkacctmgmt ]
        Parameter #4 [ <optional> $servicename ]
      }
    }
    Function [ <internal:pam> function pam_chpass ] {

      - Parameters [5] {
        Parameter #0 [ <required> $username ]
        Parameter #1 [ <required> $oldpassword ]
        Parameter #2 [ <required> $newpassword ]
        Parameter #3 [ <optional> &$status ]
        Parameter #4 [ <optional> $servicename ]
      }
    }

With this proposal

    Function [ <internal:pam> function pam_auth ] {

      - Parameters [5] {
        Parameter #0 [ <required> string $username ]
        Parameter #1 [ <required> string $password ]
        Parameter #2 [ <optional> &$status = null ]
        Parameter #3 [ <optional> bool $checkacctmgmt = true ]
        Parameter #4 [ <optional> string $servicename = null ]
      }
      - Return [ bool ]
    }
    Function [ <internal:pam> function pam_chpass ] {

      - Parameters [5] {
        Parameter #0 [ <required> string $username ]
        Parameter #1 [ <required> string $oldpassword ]
        Parameter #2 [ <required> string $newpassword ]
        Parameter #3 [ <optional> &$status = null ]
        Parameter #4 [ <optional> string $servicename = null ]
      }
      - Return [ bool ]
    }

Notice: All 3 new files need to be added in package.xml

amishmm commented 3 years ago

Sorry new to this stub thing, but when I try:

php /usr/lib/php/build/gen_stub.php pam.stub.php

I get this error.

PHP Warning: fopen(/usr/lib/php/build/PHP-Parser-install-lock): Failed to open stream: Permission denied in /usr/lib/php/build/gen_stub.php on line 1747 PHP Fatal error: Uncaught TypeError: flock(): Argument # 1 ($stream) must be of type resource, bool given in /usr/lib/php/build/gen_stub.php:1748 Stack trace: # 0 /usr/lib/php/build/gen_stub.php(1748): flock() # 1 /usr/lib/php/build/gen_stub.php(1798): installPhpParser() # 2 /usr/lib/php/build/gen_stub.php(55): initPhpParser() # 3 /usr/lib/php/build/gen_stub.php(1832): processStubFile() # 4 {main} thrown in /usr/lib/php/build/gen_stub.php on line 1748

What am I doing wrong? Why is it trying to create lock file on (/usr/lib/php/build/PHP-Parser-install-lock) instead of current directory?

How to generate arginfo.h files?

amishmm commented 3 years ago

I copied /usr/lib/php/build/gen_stub.php to project directory and ran it and it worked.

But it downloaded PHP-Parser and generated arginfo.h files.

How do I avoid these extra steps of copying gen_stub.php and also downloading of PHP-Parser?

I am sure these can be avoided but I do not know anything about this stub thing,

remicollet commented 3 years ago

Sorry, I badly read inital question.

php /usr/lib/php/build/gen_stub.php pam.stub.php

You don't have to run this

A simple make will do it properly

$ touch pam.stub.php
$ make
Parse /work/GIT/pecl-and-ext/pam/pam.stub.php to generate /work/GIT/pecl-and-ext/pam/pam_arginfo.h
...

P.S. gen_stub.php is copied during phpize

remicollet commented 3 years ago

Le 08/06/2021 à 06:19, Amish a écrit :

I did this:

phpize
./configure --prefix=/usr
make

Then make ran and first generated pam_arginfo.h file and then deleted it.

Parse /foo/php-pam/github/pam.stub.php to generate /foo/php-pam/github/pam_arginfo.h ... Build complete. Don't forget to run 'make test'.

rm /foo/php-pam/github/pam_arginfo.h

If not exists, make consider it as a build artefact and delete it during cleanup

If exist, it won't be deleted

$ ls *arginfo*
pam_legacy_arginfo.h

I looked at Makefile and could not find anything that will delete pam_arginfo.h.

So why is it deleting it and where?

remicollet commented 3 years ago

Example

$ sed -e s/username/user/ -i pam.stub.php

$ make
Parse /work/GIT/pecl-and-ext/pam/pam.stub.php to generate /work/GIT/pecl-and-ext/pam/pam_arginfo.h
Saved /work/GIT/pecl-and-ext/pam/pam_arginfo.h
Saved /work/GIT/pecl-and-ext/pam/pam_legacy_arginfo.h
...

$ git diff
diff --git a/pam.stub.php b/pam.stub.php
index a25f793..37790bf 100644
--- a/pam.stub.php
+++ b/pam.stub.php
@@ -6,8 +6,8 @@
  */

 /** @param string $status */
-function pam_auth(string $username, string $password, &$status = null, bool $checkacctmgmt = true, string $servicename = null): bool {}
+function pam_auth(string $user, string $password, &$status = null, bool $checkacctmgmt = true, string $servicename = null): bool {}

 /** @param string $status */
-function pam_chpass(string $username, string $oldpassword, string $newpassword, &$status = null, string $servicename = null): bool {}
+function pam_chpass(string $user, string $oldpassword, string $newpassword, &$status = null, string $servicename = null): bool {}

diff --git a/pam_arginfo.h b/pam_arginfo.h
index 4a63282..9226f08 100644
--- a/pam_arginfo.h
+++ b/pam_arginfo.h
@@ -1,8 +1,8 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 5d7edd77ec802eac610edf6086a09ea41aa67afb */
+ * Stub hash: f65a93a157cbd8d477c6d311843f7a8a88b08352 */

 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pam_auth, 0, 2, _IS_BOOL, 0)
-       ZEND_ARG_TYPE_INFO(0, username, IS_STRING, 0)
+       ZEND_ARG_TYPE_INFO(0, user, IS_STRING, 0)
        ZEND_ARG_TYPE_INFO(0, password, IS_STRING, 0)
        ZEND_ARG_INFO_WITH_DEFAULT_VALUE(1, status, "null")
        ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, checkacctmgmt, _IS_BOOL, 0, "true")
@@ -10,7 +10,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pam_auth, 0, 2, _IS_BOOL, 0)
 ZEND_END_ARG_INFO()

 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pam_chpass, 0, 3, _IS_BOOL, 0)
-       ZEND_ARG_TYPE_INFO(0, username, IS_STRING, 0)
+       ZEND_ARG_TYPE_INFO(0, user, IS_STRING, 0)
        ZEND_ARG_TYPE_INFO(0, oldpassword, IS_STRING, 0)
        ZEND_ARG_TYPE_INFO(0, newpassword, IS_STRING, 0)
        ZEND_ARG_INFO_WITH_DEFAULT_VALUE(1, status, "null")
diff --git a/pam_legacy_arginfo.h b/pam_legacy_arginfo.h
index 5a3eece..8e76af0 100644
--- a/pam_legacy_arginfo.h
+++ b/pam_legacy_arginfo.h
@@ -1,8 +1,8 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 5d7edd77ec802eac610edf6086a09ea41aa67afb */
+ * Stub hash: f65a93a157cbd8d477c6d311843f7a8a88b08352 */

 ZEND_BEGIN_ARG_INFO_EX(arginfo_pam_auth, 0, 0, 2)
-       ZEND_ARG_INFO(0, username)
+       ZEND_ARG_INFO(0, user)
        ZEND_ARG_INFO(0, password)
        ZEND_ARG_INFO(1, status)
        ZEND_ARG_INFO(0, checkacctmgmt)
@@ -10,7 +10,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_pam_auth, 0, 0, 2)
 ZEND_END_ARG_INFO()

 ZEND_BEGIN_ARG_INFO_EX(arginfo_pam_chpass, 0, 0, 3)
-       ZEND_ARG_INFO(0, username)
+       ZEND_ARG_INFO(0, user)
        ZEND_ARG_INFO(0, oldpassword)
        ZEND_ARG_INFO(0, newpassword)
        ZEND_ARG_INFO(1, status)

Which is exactly what is expected

amishmm commented 3 years ago

Thanks for explanation. PR is merged. Thank you.